Merged changelog has wrong changelog entry

[marandaneto]

Every merged PR with a changelog entry within a release might end up in the wrong changelog version entry.

Steps:

1. Cut a release from main, it creates the release branch.
   2, Do not approve the release yet.
2. Merge a PR into the main branch with a changelog entry.
3. Approve the release that merges the release branch into the main branch.
4. 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

[BYK]

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:

craft/src/utils/git.ts

Line 46 in 6d523c5

to: 'HEAD',

To point it to the actual rev/branch that is being released instead of HEAD.

[marandaneto]

@BYK the release was done via getsentry/publish.

[BYK]

Okay so identified the issue here. It is just a merge error, nothing to do with Craft itself. This happens in getsentry/sentry-react-native@56c9319.

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

[marandaneto]

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.
getsentry/sentry-react-native@16cfa52
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.

[kamilogorek]

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

[marandaneto]

@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.

[BYK]

@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:

craft/src/utils/changelog.ts

Line 14 in 6d523c5

export const SKIP_CHANGELOG_MAGIC_WORD = '#skip-changelog';

#knowyourtools :)

[BYK]

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:

craft/src/commands/publish.ts

Line 455 in 6d523c5

await git.checkout(checkoutTarget);

I cannot find a path that would lead our publish pipeline checkout the merge to master (as this happens once all targets are published).

[marandaneto]

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]

@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:

craft/src/utils/changelog.ts

Line 14 in 6d523c5

export const SKIP_CHANGELOG_MAGIC_WORD = '#skip-changelog';

#knowyourtools :)

Thanks @BYK
I will discuss that with @bruno-garcia
We already use #skip-changelog, so that would likely do.