pants icon indicating copy to clipboard operation
pants copied to clipboard

Require release notes or explicit opt-out in every PR

Open huonw opened this issue 9 months ago • 1 comments

This is CI in service of continually maintaining our detailed changelog, i.e. have long form descriptions of unreleased features listed in the "latest" docs/notes/2.*.x.md file, so we don't have to write this in a batch as part of prepping the release.

The detailed changelog for a big release is a better for users than listing PR titles (as we do for each incremental dev release), because it gives space for linking to docs, giving code examples etc.

This PR adds CI to enforce that every PR, either:

  • changes a docs/notes/ file, usually in the form of adding a paragraph or two to docs/notes/2.22.x.md (or similar, whatever the latest main release is)
  • explicitly opts out via a label (category:internal, or the yet-to-be-created release-notes:not-required)

This PR tries to be a minimal MVP of a process improvement.

The key bit of this CI is enforcing an explicit opt-out: we can say "let's maintain a changelog as we go" without this PR, but I think we'll forget to do it all the time, if we don't have a computer reminding us.

Example of how this works in practice

Feature or fix worth including in release notes

  1. Someone opens a PR
  2. The release notes CI step fails
  3. The author or reviewer notices this and suggests that release notes be included in the appropriate docs/notes/2.*.x.md file
  4. The PR is updated with release notes, CI reruns and passes now

Minor change not worth including in release notes

  1. Someone opens a PR
  2. The release notes CI step fails
  3. The author or reviewer notices this and a maintainer tags the PR with release-notes:not-required
  4. The release notes CI step can be rerun and passes now

Demo of error

Demonstration of a failure when this PR was tagged with category:new feature (instead of category:internal): https://github.com/pantsbuild/pants/actions/runs/8905804902/job/24457010553?pr=20850

Problems

There's a few ways this can go wrong:

  1. someone adds notes to the wrong file by mistake: up to a reviewer to catch, but we could add active checks for it too (i.e. we could be explicitly enforcing that there should be changes to the current release notes).
  2. if a PR has been open for a long time, it might make changes to the 2.22.x.md file, but only merge in time for 2.23.x, and thus the release notes are targetting the wrong series. There's a few potential mitigations for this (I think 1 is likely good enough for now):
    1. when branching for a new release, the person doing that can go and check open PRs for changes to the wrong release notes file (potentially there's some magic script I can write to make this easy) and ask the author to update
    2. if merged via a merge queue (e.g. https://github.com/pantsbuild/pants/discussions/20193), a CI check for 1 would catch this too
  3. this'll make cherry-picking more awkward and fail more often, e.g. if a change has release note in 2.22.x.md and we cherry pick it to the 2.21.x branch, that file won't exist and the cherry picker will fail. I think this is probably tolerable for now (at least, worth trying). There's a potential few mitigations (I think the first is the better use of energy):
    1. switching to a different approach to notes where they end up in separate dedicated files (see "Future work")
    2. making the cherry-picker script more intelligent, e.g. ignore notes or copy them across magically

Initial roll-out

We're early in the 2.22.x dev cycle on main. Assuming we have agreement on the approach, I'll whip up a 2.22.x.md notes file with everything that's landed before this, and then future PRs will augment that.

Once we branch for 2.22.x and start on 2.23.x we'll just need to make the 2.23.x.md file then, and keep going as we were.

Future work

There's further iteration we can do on this:

  • We may want to eventually consider a tool like towncrier (as discussed https://github.com/pantsbuild/pants/discussions/19247), especially if we're getting a lot of merge conflicts on the release notes files, since that'll aggregate the notes across separate files that don't interact between PRs.
  • Putting these human-curated notes into the GitHub release announcements, rather than the PR titles.

However, for either of those, I think we'll still (conceptually) want enforcement along these lines: a PR needs to either have release notes included (in whatever format we land on), or a human needs to explicitly opt-out. Thus, this is an incremental step that won't be wasted work.

huonw avatar Apr 27 '24 04:04 huonw

Great idea, thanks for writing it up.

I like using both category:internal and release-notes:not-required for this.

benjyw avatar May 01 '24 11:05 benjyw

I've added some docs, and started the release notes file for 2.22 in #20878

huonw avatar May 07 '24 01:05 huonw