Mapbox-gl-js: Create a system to enforce that PR authors are responsible for updating the changelog

[lucaswoj]

The pace of development and complexity of GL JS makes it time consuming and error prone for the engineer doing the release to write the whole change log by hand at release time.

We could consider following the lead of other teams at Mapbox and require that PR authors update the change log if their changes affect end users or explicitly opt out from updating the change log if the changes do not affect end users.

[andrewharvey]

Do you mean that each PR would include an entry in the CHANGELOG under a master heading (if worthy enough to have an entry) and then at release time the master heading is changed over to the version?

I'm :+1: for this. I've always preferred to update the CHANGELOG as part of each PR, but for my PR's I haven't simply because it wasn't being done that way.

Added bonus is people can see what changes are coming soon in the next release.

[jfirebaugh]

Downside of this is it triggers a merge conflict in almost every PR.

[mollymerp]

there are a fair number of PRs that would not warrant an update in the changelog (changes to testing patterns or internal refactors that don't affect end users). I'd prefer a checkbox on the PR template to a hard requirement 🤷‍♀️

[anandthakker]

The underlying problem here is that manually building the changelog from the git history each time we do a release can be quite difficult and time consuming.

An alternative solution to requiring a changelog update in each PR might be to use some sort of system for commit comments, a la https://commitizen.github.io/cz-cli/, to make it easier to build the changelog.

[lucaswoj]

I'm loving all these ideas! Just edited the issue title to increase the design space a little.

[1ec5]

Good release notes require some manual curation, whether it’s at release time or periodically during the release cycle. Here are some examples I’ve seen from gardening the iOS SDK’s release notes:

• Developers reading the release notes don’t need to know about fixes for regressions that were introduced within the same release cycle.
• Breaking changes should be highlighted and ideally listed up-front.
• Changes should be sorted in order of developer impact.
• Long release notes can be broken up by topic area (each topic sorted by developer impact) to make the notes more scannable. Sometimes the topics aren’t known in advance. Dividing the notes by topic can also reduce the occurrence of merge conflicts.
• Whereas the commit message needs to summarize the code-level changes (“added missing calls to foo()”), the developer impact may be distinct (“fixed bug preventing redraws when pressing bar”).

Commitzen can certainly make it easier to perform that curation, but the biggest improvement would come from treating CHANGELOG.md as a draft release notes and curating it multiple times during a release cycle, while the context is still fresh.

[1ec5]

As for a system of enforcement, I’d recommend Danger, which works by commenting on the PR instead of updating webhooks. Danger can leave a reminder as a warning without failing the build.

[jfirebaugh]

https://github.com/isaacs/github/issues/487 / https://github.com/isaacs/github/issues/560

[anandthakker]

Considering the merge conflict issue @jfirebaugh mentions above, and @1ec5 's point about manual curation, I propose that we attempt to streamline--but not fully automate--this process by doing the following:

1. Create a script that reads the git log since the last release and transforms it into a list of PR/issue entries like: [Number and title of PR] [github link] [list of associated issues, scraped from ‘Fixes #…’ PR / commit comments. With this list serving as a rough draft of the changelog entry for a release, we'd eliminate the tedium of chasing down PRs and issues for each commit and instead be able to focus on the part that's actually important: communicating clearly the changes in this release.

2. Add a Danger bot that helps us to enforce the standard that every PR description contains either: Changelog: n/a (reason) or Changelog: breaking|feature|fix [description of change, if different from PR title]

1 is more important than 2, but I think 2 is a good idea because it essentially asks the the author of a change to handle thinking about how best to communicate it, thus avoiding the situation where the person running the release doesn't have enough context to do so and has to chase down the author to figure out what to say.

[1ec5]

Edit: I just realized this ticket is in the gl-js repository rather than the gl-native repository. Never mind me. 😅

Manual curation is one of the benefits of the changelog workflow that the iOS team practices (and tries to help the core team practice). The other benefit is having a running changelog in one place in source control. If we rely on the script to aggregate these release notes, then either we need a page on mapbox.github.io that gets updated regularly with that script’s output, or we need to do a manual weekly roundup that checks in these notes. Ultimately, the release notes do need to work their way into CHANGELOG.md, since that’s what we use to generate the cover page of the jazzy docset.

If you haven’t already, please take a look at the iOS and macOS release note template and the iOS changelog. Note how we categorize the changes by topic and sort them by developer or user impact rather than chronologically – that’s been our approach to cutting down on merge conflicts.

These release notes apply much of Mapbox’s documentation style guide, including the use of complete sentences. Hopefully we can maintain that high quality standard as we streamline the workflow.

[anandthakker]

The other benefit is having a running changelog in one place in source control.

@1ec5 my intent in https://github.com/mapbox/mapbox-gl-js/issues/5670#issuecomment-386706814 was that we'd continue to track CHANGELOG.md in source control and update it at the time of each release (usually in the vX.Y.Z commit). The script output would simply be used to give the person making that update a head start in the manual curation work.

or we need to do a manual weekly roundup that checks in these notes

Why weekly, rather than just with each release?

Note how we categorize the changes by topic and sort them by developer or user impact rather than chronologically – that’s been our approach to cutting down on merge conflicts

👍 to categorization and sorting by impact. Good point about cutting down merge conflicts -- but there's still likely to be conflict on the 2nd and 3rd change in each category, right?

[1ec5]

I posted https://github.com/mapbox/mapbox-gl-js/issues/5670#issuecomment-386711535 before realizing this isn’t the gl-native repository, where we already maintain a running changelog on each merge but sometimes miss stuff that happens in mbgl. Sorry for the confusion.

[anandthakker]

Quick first cut at this changelog-draft script here: https://github.com/mapbox/github-release-tools

Here's an example of what it currently generates: https://gist.github.com/anandthakker/a913b632c719f57a4bf6e8356943f868

[andrewharvey]

Quick first cut at this changelog-draft script here: https://github.com/mapbox/github-release-tools

Looks good.

I noticed that the hat tipping logic is based on the PR coming from a fork, I'm not sure how many external contributors you have with commit access, but right now it won't automatically hat tip their PRs. It's not an issue for my contributions, but something to keep in mind for others...

[andrewharvey]

9023 proposes an option to tag the changeset category with a github label and use the PR title or <changelog> as the changelog entry

[andrewharvey]

I'll close this issue since #9023 is in place allowing PR authors to set the changelog entry and label for category (which I believe then get's auto populated into the changelog at release time).

If this still doesn't address the request, feel free to re-open.