Generate a release notes page per patch version (#85279)

Closes #85250. The approach of generating a single asciidoc page for all releases in a minor series makes it harder to preserve any manual edits that we have to make. Instead, generate a page per-version. It should make little difference to the final documentation that users see.
elastic · Mar 23, 2022 · c693f8c · c693f8c
1 parent 6563e01
commit c693f8c
Show file tree

Hide file tree

Showing 12 changed files with 138 additions and 225 deletions.
diff --git a/...nal/src/main/java/org/elasticsearch/gradle/internal/release/GenerateReleaseNotesTask.java b/...nal/src/main/java/org/elasticsearch/gradle/internal/release/GenerateReleaseNotesTask.java
@@ -82,15 +82,17 @@ public GenerateReleaseNotesTask(ObjectFactory objectFactory, ExecOperations exec
 
     @TaskAction
     public void executeTask() throws IOException {
-        if (needsGitTags(VersionProperties.getElasticsearch())) {
+        final String currentVersion = VersionProperties.getElasticsearch();
+
+        if (needsGitTags(currentVersion)) {
             findAndUpdateUpstreamRemote(gitWrapper);
         }
 
         LOGGER.info("Finding changelog files...");
 
         final Map<QualifiedVersion, Set<File>> filesByVersion = partitionFilesByVersion(
             gitWrapper,
-            VersionProperties.getElasticsearch(),
+            currentVersion,
             this.changelogs.getFiles()
         );
 
@@ -103,7 +105,7 @@ public void executeTask() throws IOException {
             changelogsByVersion.put(version, entriesForVersion);
         });
 
-        final Set<QualifiedVersion> versions = getVersions(gitWrapper, VersionProperties.getElasticsearch());
+        final Set<QualifiedVersion> versions = getVersions(gitWrapper, currentVersion);
 
         LOGGER.info("Updating release notes index...");
         ReleaseNotesIndexGenerator.update(
@@ -113,10 +115,12 @@ public void executeTask() throws IOException {
         );
 
         LOGGER.info("Generating release notes...");
+        final QualifiedVersion qualifiedVersion = QualifiedVersion.of(currentVersion);
         ReleaseNotesGenerator.update(
             this.releaseNotesTemplate.get().getAsFile(),
             this.releaseNotesFile.get().getAsFile(),
-            changelogsByVersion
+            qualifiedVersion,
+            changelogsByVersion.getOrDefault(qualifiedVersion, Set.of())
         );
 
         LOGGER.info("Generating release highlights...");

diff --git a/...ls-internal/src/main/java/org/elasticsearch/gradle/internal/release/QualifiedVersion.java b/...ls-internal/src/main/java/org/elasticsearch/gradle/internal/release/QualifiedVersion.java
@@ -11,6 +11,7 @@
 import org.elasticsearch.gradle.Version;
 
 import java.util.Comparator;
+import java.util.Locale;
 import java.util.Objects;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -21,12 +22,7 @@
  * with how {@link Version} is used in the build. It also retains any qualifier (prerelease) information, and uses
  * that information when comparing instances.
  */
-public record QualifiedVersion(
-    int major,
-    int minor,
-    int revision,
-    org.elasticsearch.gradle.internal.release.QualifiedVersion.Qualifier qualifier
-) implements Comparable<QualifiedVersion> {
+public record QualifiedVersion(int major, int minor, int revision, Qualifier qualifier) implements Comparable<QualifiedVersion> {
 
     private static final Pattern pattern = Pattern.compile(
         "^v? (\\d+) \\. (\\d+) \\. (\\d+) (?: - (alpha\\d+ | beta\\d+ | rc\\d+ | SNAPSHOT ) )? $",
@@ -56,7 +52,7 @@ public static QualifiedVersion of(final String s) {
 
     @Override
     public String toString() {
-        return "%d.%d.%d%s".formatted(major, minor, revision, qualifier == null ? "" : "-" + qualifier);
+        return String.format(Locale.ROOT, "%d.%d.%d%s", major, minor, revision, qualifier == null ? "" : "-" + qualifier);
     }
 
     public boolean hasQualifier() {

diff --git a/...ternal/src/main/java/org/elasticsearch/gradle/internal/release/ReleaseNotesGenerator.java b/...ternal/src/main/java/org/elasticsearch/gradle/internal/release/ReleaseNotesGenerator.java
@@ -14,7 +14,6 @@
 import java.io.FileWriter;
 import java.io.IOException;
 import java.nio.file.Files;
-import java.util.Comparator;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -31,7 +30,7 @@
  */
 public class ReleaseNotesGenerator {
     /**
-     * These mappings translate change types into the headings as they should appears in the release notes.
+     * These mappings translate change types into the headings as they should appear in the release notes.
      */
     private static final Map<String, String> TYPE_LABELS = new HashMap<>();
 
@@ -47,63 +46,48 @@ public class ReleaseNotesGenerator {
         TYPE_LABELS.put("upgrade", "Upgrades");
     }
 
-    static void update(File templateFile, File outputFile, Map<QualifiedVersion, Set<ChangelogEntry>> changelogs) throws IOException {
+    static void update(File templateFile, File outputFile, QualifiedVersion version, Set<ChangelogEntry> changelogs) throws IOException {
         final String templateString = Files.readString(templateFile.toPath());
 
         try (FileWriter output = new FileWriter(outputFile)) {
-            output.write(generateFile(templateString, changelogs));
+            output.write(generateFile(templateString, version, changelogs));
         }
     }
 
     @VisibleForTesting
-    static String generateFile(String template, Map<QualifiedVersion, Set<ChangelogEntry>> changelogs) throws IOException {
-        final var changelogsByVersionByTypeByArea = buildChangelogBreakdown(changelogs);
+    static String generateFile(String template, QualifiedVersion version, Set<ChangelogEntry> changelogs) throws IOException {
+        final var changelogsByTypeByArea = buildChangelogBreakdown(changelogs);
 
         final Map<String, Object> bindings = new HashMap<>();
-        bindings.put("changelogsByVersionByTypeByArea", changelogsByVersionByTypeByArea);
+        bindings.put("version", version);
+        bindings.put("changelogsByTypeByArea", changelogsByTypeByArea);
         bindings.put("TYPE_LABELS", TYPE_LABELS);
 
         return TemplateUtils.render(template, bindings);
     }
 
-    private static Map<QualifiedVersion, Map<String, Map<String, List<ChangelogEntry>>>> buildChangelogBreakdown(
-        Map<QualifiedVersion, Set<ChangelogEntry>> changelogsByVersion
-    ) {
-        Map<QualifiedVersion, Map<String, Map<String, List<ChangelogEntry>>>> changelogsByVersionByTypeByArea = new TreeMap<>(
-            Comparator.reverseOrder()
-        );
-
-        changelogsByVersion.forEach((version, changelogs) -> {
-            Map<String, Map<String, List<ChangelogEntry>>> changelogsByTypeByArea = changelogs.stream()
-                .collect(
+    private static Map<String, Map<String, List<ChangelogEntry>>> buildChangelogBreakdown(Set<ChangelogEntry> changelogs) {
+        Map<String, Map<String, List<ChangelogEntry>>> changelogsByTypeByArea = changelogs.stream()
+            .collect(
+                groupingBy(
+                    // Entries with breaking info are always put in the breaking section
+                    entry -> entry.getBreaking() == null ? entry.getType() : "breaking",
+                    TreeMap::new,
+                    // Group changelogs for each type by their team area
                     groupingBy(
-                        // Entries with breaking info are always put in the breaking section
-                        entry -> entry.getBreaking() == null ? entry.getType() : "breaking",
+                        // `security` and `known-issue` areas don't need to supply an area
+                        entry -> entry.getType().equals("known-issue") || entry.getType().equals("security") ? "_all_" : entry.getArea(),
                         TreeMap::new,
-                        // Group changelogs for each type by their team area
-                        groupingBy(
-                            // `security` and `known-issue` areas don't need to supply an area
-                            entry -> entry.getType().equals("known-issue") || entry.getType().equals("security")
-                                ? "_all_"
-                                : entry.getArea(),
-                            TreeMap::new,
-                            toList()
-                        )
+                        toList()
                     )
-                );
-
-            changelogsByVersionByTypeByArea.put(version, changelogsByTypeByArea);
-        });
+                )
+            );
 
         // Sort per-area changelogs by their summary text. Assumes that the underlying list is sortable
-        changelogsByVersionByTypeByArea.forEach(
-            (_version, byVersion) -> byVersion.forEach(
-                (_type, byTeam) -> byTeam.forEach(
-                    (_team, changelogsForTeam) -> changelogsForTeam.sort(comparing(ChangelogEntry::getSummary))
-                )
-            )
+        changelogsByTypeByArea.forEach(
+            (_type, byTeam) -> byTeam.forEach((_team, changelogsForTeam) -> changelogsForTeam.sort(comparing(ChangelogEntry::getSummary)))
         );
 
-        return changelogsByVersionByTypeByArea;
+        return changelogsByTypeByArea;
     }
 }
diff --git a/...l/src/main/java/org/elasticsearch/gradle/internal/release/ReleaseNotesIndexGenerator.java b/...l/src/main/java/org/elasticsearch/gradle/internal/release/ReleaseNotesIndexGenerator.java
@@ -43,7 +43,12 @@ static String generateFile(Set<QualifiedVersion> versionsSet, String template) t
         versionsSet.stream().map(v -> v.isSnapshot() ? v.withoutQualifier() : v).forEach(versions::add);
 
         final List<String> includeVersions = versions.stream()
-            .map(v -> v.hasQualifier() ? v.toString() : v.major() + "." + v.minor())
+            .map(
+                // We didn't split up the notes for 8.0
+                version -> version.isBefore(QualifiedVersion.of("8.1.0")) && version.hasQualifier() == false
+                    ? version.major() + "." + version.minor()
+                    : version.toString()
+            )
             .distinct()
             .collect(Collectors.toList());
 

diff --git a/...-internal/src/main/java/org/elasticsearch/gradle/internal/release/ReleaseToolsPlugin.java b/...-internal/src/main/java/org/elasticsearch/gradle/internal/release/ReleaseToolsPlugin.java
@@ -77,7 +77,14 @@ public void apply(Project project) {
 
             task.setReleaseNotesTemplate(projectDirectory.file(RESOURCES + "templates/release-notes.asciidoc"));
             task.setReleaseNotesFile(
-                projectDirectory.file(String.format("docs/reference/release-notes/%d.%d.asciidoc", version.getMajor(), version.getMinor()))
+                projectDirectory.file(
+                    String.format(
+                        "docs/reference/release-notes/%d.%d.%d.asciidoc",
+                        version.getMajor(),
+                        version.getMinor(),
+                        version.getRevision()
+                    )
+                )
             );
 
             task.setReleaseHighlightsTemplate(projectDirectory.file(RESOURCES + "templates/release-highlights.asciidoc"));

diff --git a/build-tools-internal/src/main/resources/templates/release-notes.asciidoc b/build-tools-internal/src/main/resources/templates/release-notes.asciidoc
@@ -1,37 +1,37 @@
-<% for (version in changelogsByVersionByTypeByArea.keySet()) {
+<%
 def unqualifiedVersion = version.withoutQualifier()
 %>[[release-notes-$unqualifiedVersion]]
 == {es} version ${unqualifiedVersion}
 <% if (version.isSnapshot()) { %>
 coming[$unqualifiedVersion]
 <% } %>
 Also see <<breaking-changes-${ version.major }.${ version.minor },Breaking changes in ${ version.major }.${ version.minor }>>.
-<% if (changelogsByVersionByTypeByArea[version]["security"] != null) { %>
+<% if (changelogsByTypeByArea["security"] != null) { %>
 [discrete]
 [[security-updates-${unqualifiedVersion}]]
 === Security updates
 
-<% for (change in changelogsByVersionByTypeByArea[version].remove("security").remove("_all_")) {
+<% for (change in changelogsByTypeByArea.remove("security").remove("_all_")) {
     print "* ${change.summary}\n"
 }
 }
-if (changelogsByVersionByTypeByArea[version]["known-issue"] != null) { %>
+if (changelogsByTypeByArea["known-issue"] != null) { %>
 [discrete]
 [[known-issues-${unqualifiedVersion}]]
 === Known issues
 
-<% for (change in changelogsByVersionByTypeByArea[version].remove("known-issue").remove("_all_")) {
+<% for (change in changelogsByTypeByArea.remove("known-issue").remove("_all_")) {
     print "* ${change.summary}\n"
 }
 }
-for (changeType in changelogsByVersionByTypeByArea[version].keySet()) { %>
+for (changeType in changelogsByTypeByArea.keySet()) { %>
 [[${ changeType }-${ unqualifiedVersion }]]
 [float]
 === ${ TYPE_LABELS.getOrDefault(changeType, 'No mapping for TYPE_LABELS[' + changeType + ']') }
-<% for (team in changelogsByVersionByTypeByArea[version][changeType].keySet()) {
+<% for (team in changelogsByTypeByArea[changeType].keySet()) {
     print "\n${team}::\n";
 
-    for (change in changelogsByVersionByTypeByArea[version][changeType][team]) {
+    for (change in changelogsByTypeByArea[changeType][team]) {
         print "* ${change.summary} {es-pull}${change.pr}[#${change.pr}]"
         if (change.issues != null && change.issues.empty == false) {
             print change.issues.size() == 1 ? " (issue: " : " (issues: "
@@ -43,5 +43,3 @@ for (changeType in changelogsByVersionByTypeByArea[version].keySet()) { %>
 }
 }
 print "\n\n"
-}
-%>
diff --git a/...al/src/test/java/org/elasticsearch/gradle/internal/release/ReleaseNotesGeneratorTest.java b/...al/src/test/java/org/elasticsearch/gradle/internal/release/ReleaseNotesGeneratorTest.java
@@ -14,10 +14,8 @@
 import java.nio.file.Files;
 import java.nio.file.Paths;
 import java.util.ArrayList;
-import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
-import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
 
@@ -37,52 +35,36 @@ public void generateFile_rendersCorrectMarkup() throws Exception {
             "/org/elasticsearch/gradle/internal/release/ReleaseNotesGeneratorTest.generateFile.asciidoc"
         );
 
-        final Map<QualifiedVersion, Set<ChangelogEntry>> entries = getEntries();
+        final Set<ChangelogEntry> entries = getEntries();
 
         // when:
-        final String actualOutput = ReleaseNotesGenerator.generateFile(template, entries);
+        final String actualOutput = ReleaseNotesGenerator.generateFile(template, QualifiedVersion.of("8.2.0-SNAPSHOT"), entries);
 
         // then:
         assertThat(actualOutput, equalTo(expectedOutput));
     }
 
-    private Map<QualifiedVersion, Set<ChangelogEntry>> getEntries() {
-        final Set<ChangelogEntry> entries_8_2_0 = new HashSet<>();
-        entries_8_2_0.addAll(buildEntries(1, 2));
-        entries_8_2_0.addAll(buildEntries(2, 2));
-        entries_8_2_0.addAll(buildEntries(3, 2));
-
-        final Set<ChangelogEntry> entries_8_1_0 = new HashSet<>();
-        entries_8_1_0.addAll(buildEntries(4, 2));
-        entries_8_1_0.addAll(buildEntries(5, 2));
-        entries_8_1_0.addAll(buildEntries(6, 2));
-
-        final Set<ChangelogEntry> entries_8_0_0 = new HashSet<>();
-        entries_8_0_0.addAll(buildEntries(7, 2));
-        entries_8_0_0.addAll(buildEntries(8, 2));
-        entries_8_0_0.addAll(buildEntries(9, 2));
+    private Set<ChangelogEntry> getEntries() {
+        final Set<ChangelogEntry> entries = new HashSet<>();
+        entries.addAll(buildEntries(1, 2));
+        entries.addAll(buildEntries(2, 2));
+        entries.addAll(buildEntries(3, 2));
 
         // Security issues are presented first in the notes
         final ChangelogEntry securityEntry = new ChangelogEntry();
         securityEntry.setArea("Security");
         securityEntry.setType("security");
         securityEntry.setSummary("Test security issue");
-        entries_8_2_0.add(securityEntry);
+        entries.add(securityEntry);
 
         // known issues are presented after security issues
         final ChangelogEntry knownIssue = new ChangelogEntry();
         knownIssue.setArea("Search");
         knownIssue.setType("known-issue");
         knownIssue.setSummary("Test known issue");
-        entries_8_1_0.add(knownIssue);
-
-        final Map<QualifiedVersion, Set<ChangelogEntry>> result = new HashMap<>();
+        entries.add(knownIssue);
 
-        result.put(QualifiedVersion.of("8.2.0-SNAPSHOT"), entries_8_2_0);
-        result.put(QualifiedVersion.of("8.1.0"), entries_8_1_0);
-        result.put(QualifiedVersion.of("8.0.0"), entries_8_0_0);
-
-        return result;
+        return entries;
     }
 
     private List<ChangelogEntry> buildEntries(int seed, int count) {