Merged changelog has wrong changelog entry
Every merged PR with a changelog entry within a release might end up in the wrong changelog version entry.
Steps:
- Cut a release from
main, it creates the release branch. 2, Do not approve the release yet. - Merge a PR into the main branch with a changelog entry.
- Approve the release that merges the release branch into the main branch.
- The changelog entry from step 3 is within the release but it's not published since this PR was merged after step 1.
The problem is that you always have to check that manually after releases, or your changelog might have wrong entries.
Image gotten from: https://github.com/getsentry/sentry-react-native/commits/main
This should never happen if you are doing the releases via getsentry/publish. It can only happen locally and the solution to that is probably changing this line: https://github.com/getsentry/craft/blob/6d523c5883eb0cadfa6ab33220d966e2ffd5fca9/src/utils/git.ts#L46
To point it to the actual rev/branch that is being released instead of HEAD.
@BYK the release was done via getsentry/publish.
Okay so identified the issue here. It is just a merge error, nothing to do with Craft itself. This happens in https://github.com/getsentry/sentry-react-native/commit/56c9319624c38be2331113ff17e2ac48db628098.
The actual Changelog and the changelog entry in the GitHub releases page are correct.
If you want to avoid these kinds of issues, using Craft's auto changelog feature is the best: https://github.com/getsentry/craft#changelog-policies
The actual Changelog and the changelog entry in the GitHub releases page are correct.
I've fixed them manually, they were wrong and that's the reason I've opened this issue. Changelog file and Github release page both included a wrong PR within the published version. https://github.com/getsentry/sentry-react-native/commit/16cfa52fcae796c51d1880b2ad6d9fc73dbede59 See item 2102
If you want to avoid these kinds of issues, using Craft's auto changelog feature is the best: getsentry/craft#changelog-policies
The problem with auto is that every merged PR is gonna be within the release, even meta changes or non user-facing changes.
We try to keep our changelog as clean as possible, so users don't need to dig into every PR or read every meta change.
I wanted to try the auto changelog a while ago but @bruno-garcia came up with the same arguments, so we're still doing it via the Unreleased entry.
That said, unless we're doing something wrong, this is still a bug and requires checking the changelog manually after every release.
auto will pick up either version thats about to be released or unreleased block first, so it wont show all merged PRs if you keep your changelog clean :)
If there's already an entry for the given version, use that Else if there is an entry named Unreleased, rename that to the given version
@kamilogorek yep, I get that, and we already use auto -> https://github.com/getsentry/sentry-react-native/blob/main/.craft.yml#L2
I thought that @BYK meant, not using the Unreleased tag at all, since we use auto + Unreleased manually.
@marandaneto @bruno-garcia
The problem with auto is that every merged PR is gonna be within the release, even meta changes or non user-facing changes. We try to keep our changelog as clean as possible, so users don't need to dig into every PR or read every meta change. I wanted to try the auto changelog a while ago but @bruno-garcia came up with the same arguments, so we're still doing it via the Unreleased entry.
If you want to keep things clean, you can use auto changelogs as they are intended: use milestones to group the work and the rest gets collected under "various fixes and improvements", which may still be relevant to people. If you are 100% sure something that should not go to the auto changelog, you can always add #skip-changelog to a commit or a PR (even after the fact that it was merged) to omit it from the changelog: https://github.com/getsentry/craft/blob/6d523c5883eb0cadfa6ab33220d966e2ffd5fca9/src/utils/changelog.ts#L14
#knowyourtools :)
I've fixed them manually, they were wrong and that's the reason I've opened this issue. Changelog file and Github release page both included a wrong PR within the published version.
I did see the Changelog file fix. I just couldn't trace down how the release page ended up incorrectly. The publish run references the commit without the merge and on publish we always checkout the revision/branch to publish: https://github.com/getsentry/craft/blob/6d523c5883eb0cadfa6ab33220d966e2ffd5fca9/src/commands/publish.ts#L455
I cannot find a path that would lead our publish pipeline checkout the merge to master (as this happens once all targets are published).
Maybe I've fixed the changelog file only and not the GH release (because it was correct), not 100% sure now since I've been checking many of the recent releases for Java/Gradle and Dart since they have new support for Craft and Publish.
@marandaneto @bruno-garcia
The problem with auto is that every merged PR is gonna be within the release, even meta changes or non user-facing changes. We try to keep our changelog as clean as possible, so users don't need to dig into every PR or read every meta change. I wanted to try the auto changelog a while ago but @bruno-garcia came up with the same arguments, so we're still doing it via the Unreleased entry.
If you want to keep things clean, you can use auto changelogs as they are intended: use milestones to group the work and the rest gets collected under "various fixes and improvements", which may still be relevant to people. If you are 100% sure something that should not go to the auto changelog, you can always add
#skip-changelogto a commit or a PR (even after the fact that it was merged) to omit it from the changelog:https://github.com/getsentry/craft/blob/6d523c5883eb0cadfa6ab33220d966e2ffd5fca9/src/utils/changelog.ts#L14
#knowyourtools :)
Thanks @BYK
I will discuss that with @bruno-garcia
We already use #skip-changelog, so that would likely do.