Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework breaking changes for new structure #80907

Merged
merged 3 commits into from
Nov 23, 2021
Merged

Rework breaking changes for new structure #80907

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2553be7
Rework breaking changes for new structure
pugnascotia Nov 22, 2021
8e8f04b
Formatting
pugnascotia Nov 23, 2021
ab79e28
Address review comments
pugnascotia Nov 23, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
122 changes: 104 additions & 18 deletions ...nal/src/main/java/org/elasticsearch/gradle/internal/release/BreakingChangesGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,56 +11,118 @@
import com.google.common.annotations.VisibleForTesting;

import org.elasticsearch.gradle.VersionProperties;
import org.gradle.api.GradleException;

import java.io.File;
import java.io.FileWriter;
import java.io.IOException;
import java.nio.file.Files;
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.TreeMap;
import java.util.TreeSet;
import java.util.stream.Collectors;

import static java.util.Comparator.comparing;
import static java.util.stream.Collectors.groupingBy;
import static java.util.stream.Collectors.toCollection;

/**
* Generates the page that lists the breaking changes and deprecations for a minor version release.
* Generates the page that contains an index into the breaking changes and lists deprecations for a minor version release,
* and the individual pages for each breaking area.
*/
public class BreakingChangesGenerator {

static void update(File templateFile, File outputFile, List<ChangelogEntry> entries) throws IOException {
try (FileWriter output = new FileWriter(outputFile)) {
// Needs to match `changelog-schema.json`
private static final List<String> BREAKING_AREAS = List.of(
"Cluster and node setting",
"Command line tool",
"Index setting",
"JVM option",
"Java API",
"Logging",
"Mapping",
"Packaging",
"Painless",
"REST API",
"System requirement",
"Transform"
);

static void update(
File indexTemplateFile,
File indexOutputFile,
File outputDirectory,
File areaTemplateFile,
List<ChangelogEntry> entries
) throws IOException {
if (outputDirectory.exists()) {
if (outputDirectory.isDirectory() == false) {
throw new GradleException("Path [" + outputDirectory + "] exists but isn't a directory!");
}
} else {
Files.createDirectory(outputDirectory.toPath());
}

try (FileWriter output = new FileWriter(indexOutputFile)) {
output.write(
generateFile(QualifiedVersion.of(VersionProperties.getElasticsearch()), Files.readString(templateFile.toPath()), entries)
generateIndexFile(
QualifiedVersion.of(VersionProperties.getElasticsearch()),
Files.readString(indexTemplateFile.toPath()),
entries
)
);
}
}

@VisibleForTesting
static String generateFile(QualifiedVersion version, String template, List<ChangelogEntry> entries) throws IOException {
String areaTemplate = Files.readString(areaTemplateFile.toPath());

final Map<Boolean, Map<String, List<ChangelogEntry.Breaking>>> breakingChangesByNotabilityByArea = entries.stream()
.map(ChangelogEntry::getBreaking)
.filter(Objects::nonNull)
.sorted(comparing(ChangelogEntry.Breaking::getTitle))
.collect(
groupingBy(
ChangelogEntry.Breaking::isNotable,
groupingBy(ChangelogEntry.Breaking::getArea, TreeMap::new, Collectors.toList())
)
);
for (String breakingArea : BREAKING_AREAS) {
final List<ChangelogEntry.Breaking> entriesForArea = entries.stream()
.map(ChangelogEntry::getBreaking)
.filter(entry -> entry != null && breakingArea.equals(entry.getArea()))
.collect(Collectors.toList());

if (entriesForArea.isEmpty()) {
continue;
}

final String outputFilename = breakingArea.toLowerCase(Locale.ROOT).replaceFirst(" and", "").replaceAll(" ", "-")
+ "-changes.asciidoc";

try (FileWriter output = new FileWriter(outputDirectory.toPath().resolve(outputFilename).toFile())) {
output.write(
generateBreakingAreaFile(
QualifiedVersion.of(VersionProperties.getElasticsearch()),
areaTemplate,
breakingArea,
entriesForArea
)
);
}
}
}

@VisibleForTesting
static String generateIndexFile(QualifiedVersion version, String template, List<ChangelogEntry> entries) throws IOException {
final Map<String, List<ChangelogEntry.Deprecation>> deprecationsByArea = entries.stream()
.map(ChangelogEntry::getDeprecation)
.filter(Objects::nonNull)
.sorted(comparing(ChangelogEntry.Deprecation::getTitle))
.collect(groupingBy(ChangelogEntry.Deprecation::getArea, TreeMap::new, Collectors.toList()));

final List<String> breakingIncludeList = entries.stream()
.filter(each -> each.getBreaking() != null)
.map(each -> each.getBreaking().getArea().toLowerCase(Locale.ROOT).replaceFirst(" and", "").replaceAll(" ", "-"))
.distinct()
.sorted()
.toList();

final Map<String, Object> bindings = new HashMap<>();
bindings.put("breakingChangesByNotabilityByArea", breakingChangesByNotabilityByArea);
bindings.put("breakingIncludeList", breakingIncludeList);
bindings.put("deprecationsByArea", deprecationsByArea);
bindings.put("isElasticsearchSnapshot", version.isSnapshot());
bindings.put("majorDotMinor", version.getMajor() + "." + version.getMinor());
Expand All @@ -70,4 +132,28 @@ static String generateFile(QualifiedVersion version, String template, List<Chang

return TemplateUtils.render(template, bindings);
}

@VisibleForTesting
static String generateBreakingAreaFile(
QualifiedVersion version,
String template,
String breakingArea,
List<ChangelogEntry.Breaking> entriesForArea
) throws IOException {
final Map<Boolean, Set<ChangelogEntry.Breaking>> breakingEntriesByNotability = entriesForArea.stream()
.collect(
groupingBy(
ChangelogEntry.Breaking::isNotable,
toCollection(() -> new TreeSet<>(comparing(ChangelogEntry.Breaking::getTitle)))
)
);

final Map<String, Object> bindings = new HashMap<>();
bindings.put("breakingArea", breakingArea);
bindings.put("breakingEntriesByNotability", breakingEntriesByNotability);
bindings.put("breakingAreaAnchor", breakingArea.toLowerCase(Locale.ROOT).replaceFirst(" and", "").replaceAll(" ", "_"));
bindings.put("majorMinor", String.valueOf(version.getMajor()) + version.getMinor());

return TemplateUtils.render(template, bindings);
}
}
21 changes: 16 additions & 5 deletions ...ools-internal/src/main/java/org/elasticsearch/gradle/internal/release/ChangelogEntry.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ public static class Breaking {
private String details;
private String impact;
private boolean notable;
private boolean essSettingChange;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is this actually used? Only place I can find is in test code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's used in the breaking-changes-area.asciidoc template.

Copy link
Contributor

@mark-vieira mark-vieira Nov 23, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see where we ever actually set this value though. Am I missing something or I guess folks are expected to explicitly add this to the changelog entry file?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, exactly. Where we make a change / add / remove a setting with an impact to ESS, we can set this new field so that this setting has the appropriate icon in the docs. That's all.


public String getArea() {
return area;
Expand Down Expand Up @@ -260,6 +261,14 @@ public String getAnchor() {
return generatedAnchor(this.title);
}

public boolean isEssSettingChange() {
return essSettingChange;
}

public void setEssSettingChange(boolean essSettingChange) {
this.essSettingChange = essSettingChange;
}

@Override
public boolean equals(Object o) {
if (this == o) {
Expand All @@ -273,23 +282,25 @@ public boolean equals(Object o) {
&& Objects.equals(area, breaking.area)
&& Objects.equals(title, breaking.title)
&& Objects.equals(details, breaking.details)
&& Objects.equals(impact, breaking.impact);
&& Objects.equals(impact, breaking.impact)
&& Objects.equals(essSettingChange, breaking.essSettingChange);
}

@Override
public int hashCode() {
return Objects.hash(area, title, details, impact, notable);
return Objects.hash(area, title, details, impact, notable, essSettingChange);
}

@Override
public String toString() {
return String.format(
"Breaking{area='%s', title='%s', details='%s', impact='%s', isNotable=%s}",
"Breaking{area='%s', title='%s', details='%s', impact='%s', notable=%s, essSettingChange=%s}",
area,
title,
details,
impact,
notable
notable,
essSettingChange
);
}
}
Expand Down Expand Up @@ -351,7 +362,7 @@ public String toString() {
}

private static String generatedAnchor(String input) {
final List<String> excludes = List.of("the", "is", "a");
final List<String> excludes = List.of("the", "is", "a", "and");

final String[] words = input.toLowerCase(Locale.ROOT)
.replaceAll("[^\\w]+", "_")
Expand Down
41 changes: 34 additions & 7 deletions ...nal/src/main/java/org/elasticsearch/gradle/internal/release/GenerateReleaseNotesTask.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
import org.gradle.api.DefaultTask;
import org.gradle.api.GradleException;
import org.gradle.api.file.ConfigurableFileCollection;
import org.gradle.api.file.Directory;
import org.gradle.api.file.DirectoryProperty;
import org.gradle.api.file.FileCollection;
import org.gradle.api.file.RegularFile;
import org.gradle.api.file.RegularFileProperty;
Expand All @@ -22,6 +24,7 @@
import org.gradle.api.model.ObjectFactory;
import org.gradle.api.tasks.InputFile;
import org.gradle.api.tasks.InputFiles;
import org.gradle.api.tasks.OutputDirectory;
import org.gradle.api.tasks.OutputFile;
import org.gradle.api.tasks.TaskAction;
import org.gradle.process.ExecOperations;
Expand Down Expand Up @@ -55,11 +58,13 @@ public class GenerateReleaseNotesTask extends DefaultTask {
private final RegularFileProperty releaseNotesTemplate;
private final RegularFileProperty releaseHighlightsTemplate;
private final RegularFileProperty breakingChangesTemplate;
private final RegularFileProperty breakingChangesAreaTemplate;

private final RegularFileProperty releaseNotesIndexFile;
private final RegularFileProperty releaseNotesFile;
private final RegularFileProperty releaseHighlightsFile;
private final RegularFileProperty breakingChangesFile;
private final RegularFileProperty breakingChangesIndexFile;
private final DirectoryProperty breakingChangesDirectory;

private final GitWrapper gitWrapper;

Expand All @@ -71,11 +76,13 @@ public GenerateReleaseNotesTask(ObjectFactory objectFactory, ExecOperations exec
releaseNotesTemplate = objectFactory.fileProperty();
releaseHighlightsTemplate = objectFactory.fileProperty();
breakingChangesTemplate = objectFactory.fileProperty();
breakingChangesAreaTemplate = objectFactory.fileProperty();

releaseNotesIndexFile = objectFactory.fileProperty();
releaseNotesFile = objectFactory.fileProperty();
releaseHighlightsFile = objectFactory.fileProperty();
breakingChangesFile = objectFactory.fileProperty();
breakingChangesIndexFile = objectFactory.fileProperty();
breakingChangesDirectory = objectFactory.directoryProperty();

gitWrapper = new GitWrapper(execOperations);
}
Expand Down Expand Up @@ -129,7 +136,9 @@ public void executeTask() throws IOException {
LOGGER.info("Generating breaking changes / deprecations notes...");
BreakingChangesGenerator.update(
this.breakingChangesTemplate.get().getAsFile(),
this.breakingChangesFile.get().getAsFile(),
this.breakingChangesIndexFile.get().getAsFile(),
this.breakingChangesDirectory.get().getAsFile(),
this.breakingChangesAreaTemplate.get().getAsFile(),
entries
);
}
Expand Down Expand Up @@ -339,11 +348,29 @@ public void setReleaseHighlightsFile(RegularFile file) {
}

@OutputFile
public RegularFileProperty getBreakingChangesFile() {
return breakingChangesFile;
public RegularFileProperty getBreakingChangesIndexFile() {
return breakingChangesIndexFile;
}

public void setBreakingChangesFile(RegularFile file) {
this.breakingChangesFile.set(file);
public void setBreakingChangesIndexFile(RegularFile file) {
this.breakingChangesIndexFile.set(file);
}

public void setBreakingChangesDirectory(Directory breakingChangesDirectory) {
this.breakingChangesDirectory.set(breakingChangesDirectory);
}

@OutputDirectory
public DirectoryProperty getBreakingChangesDirectory() {
return breakingChangesDirectory;
}

@InputFile
public RegularFileProperty getBreakingChangesAreaTemplate() {
return breakingChangesAreaTemplate;
}

public void setBreakingChangesAreaTemplate(RegularFile file) {
this.breakingChangesAreaTemplate.set(file);
}
}
6 changes: 5 additions & 1 deletion ...-internal/src/main/java/org/elasticsearch/gradle/internal/release/ReleaseToolsPlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,11 +84,15 @@ public void apply(Project project) {
task.setReleaseHighlightsFile(projectDirectory.file("docs/reference/release-notes/highlights.asciidoc"));

task.setBreakingChangesTemplate(projectDirectory.file(RESOURCES + "templates/breaking-changes.asciidoc"));
task.setBreakingChangesFile(
task.setBreakingChangesIndexFile(
projectDirectory.file(
String.format("docs/reference/migration/migrate_%d_%d.asciidoc", version.getMajor(), version.getMinor())
)
);
task.setBreakingChangesAreaTemplate(projectDirectory.file(RESOURCES + "templates/breaking-changes-area.asciidoc"));
task.setBreakingChangesDirectory(
projectDirectory.dir(String.format("docs/reference/migration/migrate_%d_%d", version.getMajor(), version.getMinor()))
);

task.dependsOn(validateChangelogsTask);
});
Expand Down
6 changes: 6 additions & 0 deletions build-tools-internal/src/main/resources/changelog-schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,9 @@
},
"notable": {
"type": "boolean"
},
"ess_setting_change": {
"type": "boolean"
}
},
"required": [
Expand All @@ -179,6 +182,9 @@
"body": {
"type": "string",
"minLength": 1
},
"ess_setting_change": {
"type": "boolean"
}
},
"required": [
Expand Down
39 changes: 39 additions & 0 deletions build-tools-internal/src/main/resources/templates/breaking-changes-area.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
[discrete]
[[breaking_${majorMinor}_${breakingAreaAnchor}]]
==== ${breakingArea}

//NOTE: The notable-breaking-changes tagged regions are re-used in the
//Installation and Upgrade Guide

TIP: {ess-setting-change}

<%
[true, false].each { isNotable ->
def breakingChanges = breakingEntriesByNotability.getOrDefault(isNotable, [])

if (breakingChanges.isEmpty() == false) {
if (isNotable) {
/* No newline here, one will be added below */
print "// tag::notable-breaking-changes[]"
}

for (breaking in breakingChanges) { %>
[[${ breaking.anchor }]]
. ${breaking.title}${ breaking.essSettingChange ? ' {ess-icon}' : '' }
[%collapsible]
====
*Details* +
${breaking.details.trim()}

*Impact* +
${breaking.impact.trim()}
====
<%
}

if (isNotable) {
print "// end::notable-breaking-changes[]\n"
}
}
}
%>
Loading