Drop version field from changelog YAML #76985
Conversation
Instead, write the release notes to a file with the full version in the name, and concatenate all files for a minor series to create the actual release note file.
…g-drop-version-field
…g-drop-version-field
…g-drop-version-field
…g-drop-version-field
pugnascotia
added
>enhancement
v8.0.0
:Delivery/Tooling
|
Pinging @elastic/es-delivery (Team:Delivery) |
mark-vieira
left a comment
mark-vieira
left a comment
There was a problem hiding this comment.
Couple of minor comments. I assume we'll follow up with a PR to HOMER to omit this from generated changelog entries?
| * with how {@link Version} is used in the build. It also retains any qualifier (prerelease) information, and uses | ||
| * that information when comparing instances. | ||
| */ | ||
| public final class QualifiedVersion implements Comparable<QualifiedVersion> { |
There was a problem hiding this comment.
The Version class already seems to capture a qualifier. Can we just add what we need to that existing class?
There was a problem hiding this comment.
Well... they're not equivalent. Version will parse and store the qualifier, but does nothing else with it. It doesn't use it in toString() or in compareTo(). It also allows free-form qualifiers in relaxed mode, which I believe we use when parsing the Docker version. I didn't want to get into the potential problems of unifying them as part of this work. I'm most worried about the toString() difference.
| * @return whether fetching git tags is required | ||
| */ | ||
| @VisibleForTesting | ||
| static boolean needsGitTags(String versionString) { |
There was a problem hiding this comment.
It's not clear to me why this is. Also, wouldn't it be better to parse the version string and just check the patch version segment?
There was a problem hiding this comment.
Ok, I understand now. For the first release in a series, we know all the changelogs are going to be for that version. Can we clarify that with some comments?
| changelogsByVersion | ||
| ); | ||
|
|
||
| LOGGER.info("Generating release highlights..."); |
There was a problem hiding this comment.
So release highlights and breaking changes include all versions?
There was a problem hiding this comment.
All patch versions, yes.
| TYPE_LABELS.put("deprecation", "Deprecations"); | ||
| TYPE_LABELS.put("enhancement", "Enhancements"); | ||
| TYPE_LABELS.put("feature", "New features"); | ||
| TYPE_LABELS.put("new-aggregation", "New aggregation"); |
There was a problem hiding this comment.
There's a unique label just for new aggs?
There was a problem hiding this comment.
Apparently. Maybe in practice it'll be combined with e.g. >feature, but my test code printed null without a mapping for it.
|
I'll prepare a PR for HOMER, then we can merge this PR as soon that the HOMER change is deployed. |
The changelog generation process currently relies on version information being present in the changelog YAML descriptors. However, this makes them difficult to update in some scenarios. For example, if a PR is merged and subsequently labelled for backporting, our automation won't update the versions in the changelog YAML. We can make the process more flexible by removing version data from the changelog YAML files, and instead inferring the versions from each changelog YAML file's existence in the git tree at each tag in the minor series. This change makes the process more ergonomic for developers, but harder to test, since I can't simply concoct YAML data for a range of versions. Instead, I've added a number of unit tests, and tried to exercise all the relevant parts. It is now an error to include `versions` the YAML file.
|
Backported to |
4 participants
The changelog generation process currently relies on version information being present in the changelog YAML descriptors. However, this makes them difficult to update in some scenarios. For example, if a PR is merged and subsequently labelled for backporting, our automation won't update the versions in the changelog YAML.
We can make the process more flexible by removing version data from the changelog YAML files, and instead inferring the versions from each changelog YAML file's existence in the git tree at each tag in the minor series.
This change makes the process more ergonomic for developers, but harder to test, since I can't simply concoct YAML data for a range of versions. Instead, I've added a number of unit tests, and tried to exercise all the relevant parts. I haven't (yet) got to the trouble of building a test git repository, but I suppose I could do that.
Also, this PR makes it an error to include
versionsthe YAML file, which probably makes it harder to merge since our automation still generates that field. We may want to actually remove that field in a subsequent PR.