Skip to content

feat(ng-dev): remove prerelease changelog entries when cutting a stable release - #260

Closed
josephperrott wants to merge 3 commits into
angular:mainfrom
josephperrott:remove-prelease-changelogs-on-stable
Closed

josephperrott wants to merge 3 commits into
angular:mainfrom
josephperrott:remove-prelease-changelogs-on-stable

Conversation

@josephperrott

Copy link
Copy Markdown
Member

When cutting a stable release, the changelog entries for the prerelease entries are removed as stable release
note entries contain the entirety from the previous stable release and the newly created stable release.

@josephperrott josephperrott added the action: merge The PR is ready for merge by the caretaker label Oct 11, 2021

@devversion devversion left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM; just one suggestion for an actual release action test.

Also there is one thing I'd like to discuss/capture in this PR. We allow the caretaker to modify the changelog when the release tool stages a release. These local modifications are now lost with the larger entry being generated (e.g. for a major). I wonder if we should do something about that?

Comment thread ng-dev/release/notes/release-notes.ts
Adding support for removing prerelease changelog entries will enable release tool to in effect merge changelog
entries during major and minor releases.
…le release

When cutting a stable release, the changelog entries for the prerelease entries are removed as stable release
note entries contain the entirety from the previous stable release and the newly created stable release.
@josephperrott
josephperrott force-pushed the remove-prelease-changelogs-on-stable branch from d0f32fe to 8b60cc6 Compare October 12, 2021 16:48
@josephperrott

Copy link
Copy Markdown
Member Author

@devversion PTAL at the test which was added.

@devversion devversion left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@josephperrott looks awesome!

Can you also please respond to the review message I added as part of my intitial review? #260 (review)

@josephperrott

Copy link
Copy Markdown
Member Author

@devversion sorry missed that initially

Also there is one thing I'd like to discuss/capture in this PR. We allow the caretaker to modify the changelog when the release tool stages a release. These local modifications are now lost with the larger entry being generated (e.g. for a major). I wonder if we should do something about that?

I think its likely something that is worth us looking at, but I feel like is not in the scope of the changes in this PR. We are not changing what is generated for the release notes, we are just cleaning up the old entries that are no longer needed to be included.

I think it is worth considering, separate from this change, whether or not we need to determine the actual created git diff for the changelog(s) and then do an action based on that.

I may be misunderstanding or missing something.

@devversion

Copy link
Copy Markdown
Member

I think its likely something that is worth us looking at, but I feel like is not in the scope of the changes in this PR. We are not changing what is generated for the release notes, we are just cleaning up the old entries that are no longer needed to be included.

That's my point. It's true that we do not change anything that is generated, but consider the following scenario: A prerelease is cut and the caretaker updated the changelog entry to fix a couple of typos in commit messages / or he added a missing breaking change notice. Now later when we cut stable, these entries are removed and with that the manual edits/additions from the caretaker as well.

So that makes me wonder if we should just not allow edits for the changelog when staging a release. This makes especially sense when you think about the Github release entry of the ReleaseNotes. The Github entry will be out-of-sync anyway when the caretaker made local edits to the staging/bump version commit.

@devversion

Copy link
Copy Markdown
Member

I think this PR is good to go though; but I think it's the perfect spot to discuss this problem because this PR introduces the problem.

@clydin

clydin commented Oct 13, 2021

Copy link
Copy Markdown
Member

I think it's a useful ability to be able to adjust the change log in the release PR (typos happen and breaking changes may need to be augmented).

What if the tool warned the user that customized entries exist so that the change log can be manually reviewed? This could be a followup and definitely not blocking this PR.

Also, would it be possible to leverage the change log content itself for a particular version (edited or otherwise) as the basis for the Github release content? This could remove the disconnect between edited change log file content and the release description.

@josephperrott

Copy link
Copy Markdown
Member Author

This PR was merged into the repository by commit 8cec60f.

josephperrott added a commit that referenced this pull request Oct 13, 2021
…le release (#260)

When cutting a stable release, the changelog entries for the prerelease entries are removed as stable release
note entries contain the entirety from the previous stable release and the newly created stable release.

PR Close #260
@josephperrott
josephperrott deleted the remove-prelease-changelogs-on-stable branch October 13, 2021 18:24
@angular-automatic-lock-bot

Copy link
Copy Markdown

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot Bot locked and limited conversation to collaborators Nov 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants