Require release notes or explicit opt-out in every PR (pantsbuild#20850)

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
`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).
1. 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 is now told to go
and check open PRs for changes to the wrong release notes file and ask
the author to update, with a script
2. if merged via a merge queue (e.g.
pantsbuild#20193), a CI check for 1
would catch this too
7. 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`. I've whipped up a
`2.22.x.md` notes file with everything that's landed before this in
pantsbuild#20878, 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.

The existing stable branches (2.19.x, 2.20.x, 2.21.x) will continue with
the old process, release notes in a batch.


## Future work

There's further iteration we can do on this:

- We may want to eventually consider a tool like towncrier (as discussed
pantsbuild#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.

.github/workflows/test.yaml

@@ -562,13 +562,35 @@ jobs:
         labels: category:new feature, category:user api change, category:plugin api change, category:performance, category:bugfix,
           category:documentation, category:internal
         mode: exactly
+  check_release_notes:
+    if: github.repository_owner == 'pantsbuild'
+    name: Ensure PR has release notes
+    needs:
+    - classify_changes
+    runs-on:
+    - ubuntu-20.04
+    steps:
+    - env:
+        GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+      if: github.event_name == 'pull_request' && !needs.classify_changes.outputs.notes
+      name: Ensure appropriate label
+      uses: mheap/github-action-required-labels@v4.0.0
+      with:
+        count: 1
+        labels: release-notes:not-required, category:internal
+        message: "\nPlease do one of:\n\n- add release notes to the appropriate file in `docs/notes`\n\n- label this PR with\
+          \ `release-notes:not-required` if it does not need them (for\n  instance, if this is fixing a minor typo in documentation)\n\
+          \n- label this PR with `category:internal` if it's an internal change\n\nFeel free to ask a maintainer for help\
+          \ if you are not sure what is appropriate!\n"
+        mode: minimum
   classify_changes:
     if: github.repository_owner == 'pantsbuild'
     name: Classify changes
     outputs:
       ci_config: ${{ steps.classify.outputs.ci_config }}
       docs: ${{ steps.classify.outputs.docs }}
       docs_only: ${{ steps.classify.outputs.docs_only }}
+      notes: ${{ steps.classify.outputs.notes }}
       other: ${{ steps.classify.outputs.other }}
       release: ${{ steps.classify.outputs.release }}
       rust: ${{ steps.classify.outputs.rust }}
@@ -586,8 +608,8 @@ jobs:
         \  # pull request: compare to the base branch, ensuring that commit exists\n  git fetch --depth=1 \"$GITHUB_EVENT_PULL_REQUEST_BASE_SHA\"\
         \n  comparison_sha=\"$GITHUB_EVENT_PULL_REQUEST_BASE_SHA\"\nfi\necho \"comparison_sha=$comparison_sha\"\n\naffected=$(git\
         \ diff --name-only \"$comparison_sha\" HEAD | python build-support/bin/classify_changed_files.py)\necho \"Affected:\"\
-        \nif [[ \"${affected}\" == \"docs\" ]]; then\n  echo \"docs_only=true\" | tee -a $GITHUB_OUTPUT\nfi\nfor i in ${affected};\
-        \ do\n  echo \"${i}=true\" | tee -a $GITHUB_OUTPUT\ndone\n"
+        \nif [[ \"${affected}\" == \"docs\" || \"${affected}\" == \"docs notes\" ]]; then\n  echo \"docs_only=true\" | tee\
+        \ -a $GITHUB_OUTPUT\nfi\nfor i in ${affected}; do\n  echo \"${i}=true\" | tee -a $GITHUB_OUTPUT\ndone\n"
   lint_python:
     if: (github.repository_owner == 'pantsbuild') && (needs.classify_changes.outputs.docs_only != 'true')
     name: Lint Python and Shell
@@ -658,6 +680,7 @@ jobs:
     needs:
     - classify_changes
     - check_labels
+    - check_release_notes
     - bootstrap_pants_linux_arm64
     - bootstrap_pants_linux_x86_64
     - bootstrap_pants_macos11_x86_64
@@ -666,6 +689,7 @@ jobs:
     - build_wheels_macos10_15_x86_64
     - build_wheels_macos11_arm64
     - check_labels
+    - check_release_notes
     - classify_changes
     - lint_python
     - test_python_linux_arm64

build-support/bin/classify_changed_files.py

@@ -17,6 +17,7 @@ class Affected(enum.Enum):
     rust = "rust"
     release = "release"
     ci_config = "ci_config"
+    notes = "notes"
     other = "other"
 
 
@@ -40,13 +41,17 @@ class Affected(enum.Enum):
     "build-support/bin/classify_changed_files.py",
     "src/python/pants_release/generate_github_workflows.py",
 ]
+_notes_globs = [
+    "docs/notes/*",
+]
 
 
 _affected_to_globs = {
     Affected.docs: _docs_globs,
     Affected.rust: _rust_globs,
     Affected.release: _release_globs,
     Affected.ci_config: _ci_config_globs,
+    Affected.notes: _notes_globs,
 }
 
 
@@ -63,8 +68,7 @@ def classify(changed_files: list[str]) -> set[Affected]:
 
 def main() -> None:
     affecteds = classify(sys.stdin.read().splitlines())
-    for affected in sorted([a.name for a in affecteds]):
-        print(affected)
+    print(" ".join(sorted(a.name for a in affecteds)))
 
 
 if __name__ == "__main__":

build-support/bin/classify_changed_files_test.py

@@ -15,7 +15,7 @@
         ],
         [
             ["docs/notes/2.16.x.md"],
-            {Affected.docs},
+            {Affected.docs, Affected.notes},
         ],
         [["src/rust/engine/path/to/file.rs"], {Affected.rust}],
         [["src/python/pants/VERSION"], {Affected.release}],

docs/docs/contributions/index.mdx

@@ -158,3 +158,20 @@ Your change will be included in the next weekly dev release, which usually happe
 
 See [Release strategy](./releases/release-strategy.mdx).
 :::
+
+## Release notes
+
+We maintain release notes as we go: every pull request should add or adjust the release notes if required. These release notes are files in `docs/notes/`, grouped by release series; for example, `docs/notes/2.22.x.md` includes the release notes for 2.22 releases.
+
+The release note file is generally grouped by "backend". If you're not sure whether to add release notes, or where to put them, or how to phrase them, feel free to:
+
+- look in other release notes files in `docs/notes` for inspiration
+- ask in `#development` on Slack
+- open a pull request and ask the reviewers
+
+New features and major bug fixes should definitely have release notes, but other changes can opt out. For example, fixes to features that aren't released or minor documentation fixes.
+
+We have guidance to walk us through this, so it's not a problem to forget. Pull request CI enforces that either:
+
+- the PR release notes, by having changes in `docs/notes/`
+- someone has opted out, by labelling the PR with `release-notes:not-required` or `category:internal` (the latter means that release notes are optional for all `category:internal` PRs).

docs/docs/contributions/releases/release-process.mdx

@@ -53,6 +53,15 @@ If there is deprecated code that must be removed, you can either:
 2. Remove it yourself, in a precursor PR.
 3. Bump the deprecation removal target back by one dev release.
 
+### `a0` - Release notes for next version
+
+When releasing an `a0` version, we need to prepare the release notes for the _next_ version:
+
+1. Create pull request adding the release notes file for the next version in `docs/notes/`, e.g. if you're releasing 2.8.0a0, create `docs/notes/2.9.x.md`.
+   1. Copy the title and template over from the prior release, e.g. `2.8.x.md`.
+   2. Delete the content, leaving just the headings, so the file is mostly empty.
+2. NB. this can merge after the release pull request, but should be available soon so that pull requests tht land in `main` can update it
+
 ### `rc` - Check for cherry-picks
 
 If this is a release candidate, ensure that pending cherry-picks have been applied in the release branch. Cherry-picks are usually applied automatically, but this may not always succeed, so [check for any pending cherry-picks](https://github.com/pantsbuild/pants/pulls?q=is%3Apr+label%3Aneeds-cherrypick+is%3Aclosed), and find the relevant ones by looking at the milestone: for instance, if doing a release for 2.16, the relevant cherry-picks are those for milestone `2.16.x` or earlier.
@@ -66,13 +75,6 @@ The process may fail in one of two ways:
 
 The release commit is the commit that bumps the VERSION string. For `dev`/`a0` releases this happens in the `main` branch, in the same commit that updates the release notes and the `CONTRIBUTORS.md` file. For `rc` and stable releases, this happens in the relevant stable branch.
 
-### `dev0` - set up the new release series
-
-If this is the first dev release in a new series:
-
-1. Create a new file in `docs/notes`, e.g. create `docs/notes/2.9.x.md`.
-   1. Copy the title and template over from the prior release, e.g. `2.8.x.md`.
-
 ### Bump the VERSION
 
 From the `main` branch, run `pants run src/python/pants_release/start_release.py -- --new 2.9.0.dev1 --release-manager your_github_username --publish` with the relevant version and your own GitHub username.
@@ -86,9 +88,14 @@ This will create a pull request that:
 
 Post the PR to the `#development` channel in Slack. Merge once approved and green.
 
-### `a0` - create a new Git branch
+### `a0` - new release cycle
+
+If you're releasing an `a0` release, you must:
 
-If you're releasing an `a0` release, you must create the stable branch for that version.
+1. create the stable branch for that version,
+2. identify pull requests that need changes to their release notes.
+
+#### Create new branch
 
 For example, if you're releasing `2.9.0a0`, create the branch `2.9.x` by running the command below. Make sure you are on your release commit before doing this.
 
@@ -97,6 +104,22 @@ $ git checkout -b 2.9.x
 $ git push upstream 2.9.x
 ```
 
+#### Identify pull requests needing changes
+
+Find unmerged pull requests with release notes changes that will now be targetting the new release, and so should be updated:
+
+1. Use this `gh` CLI command to find pull requests targetting `main` that touch any release notes file:
+
+   ```shell
+   gh pr list --limit 1000 --json url,files --base main | jq 'map(select(.files | map(.path | startswith("docs/notes/")) | any) | .url)'
+   ```
+
+2. For each of them, add a "Request changes" review that asks for an update. Suggested review text (replace `$OLD_VERSION` and `$NEW_VERSION` as appropriate, e.g. if you're releasing 2.8.0a0, `OLD_VERSION = 2.8`, `NEW_VERSION = 2.9`):
+
+   > Thanks for the contribution. We've just branched for $OLD_VERSION, so merging this pull request now will come out in $NEW_VERSION, please move the release notes updates to `docs/notes/$NEW_VERSION.x.md` if that's appropriate.
+
+3. In some cases, the release note changes can remain in the old version (for example, if the pull request is a bug fix that needs to be cherry-picked).
+
 ## Step 2: Tag the release to trigger publishing
 
 Once you have merged the `VERSION` bump — which will be on `main` for `dev` and `a0` releases, and on the release branch for release candidates — tag the release commit to trigger wheel building and publishing.

src/python/pants_release/generate_github_workflows.py

@@ -109,6 +109,7 @@ def classify_changes() -> Jobs:
                 "rust": gha_expr("steps.classify.outputs.rust"),
                 "release": gha_expr("steps.classify.outputs.release"),
                 "ci_config": gha_expr("steps.classify.outputs.ci_config"),
+                "notes": gha_expr("steps.classify.outputs.notes"),
                 "other": gha_expr("steps.classify.outputs.other"),
             },
             "steps": [
@@ -131,7 +132,7 @@ def classify_changes() -> Jobs:
 
                         affected=$(git diff --name-only "$comparison_sha" HEAD | python build-support/bin/classify_changed_files.py)
                         echo "Affected:"
-                        if [[ "${affected}" == "docs" ]]; then
+                        if [[ "${affected}" == "docs" || "${affected}" == "docs notes" ]]; then
                           echo "docs_only=true" | tee -a $GITHUB_OUTPUT
                         fi
                         for i in ${affected}; do
@@ -168,6 +169,41 @@ def ensure_category_label() -> Sequence[Step]:
     ]
 
 
+def ensure_release_notes() -> Sequence[Step]:
+    """Check that a PR either has release notes, or a category:internal or release-notes:not-
+    required label."""
+    return [
+        {
+            # If there's release note changes, then we're good to go and no need to check for one of
+            # the opt-out labels. If there's not, then we should check to see if a human has opted
+            # out via a label.
+            "if": "github.event_name == 'pull_request' && !needs.classify_changes.outputs.notes",
+            "name": "Ensure appropriate label",
+            "uses": "mheap/github-action-required-labels@v4.0.0",
+            "env": {"GITHUB_TOKEN": gha_expr("secrets.GITHUB_TOKEN")},
+            "with": {
+                "mode": "minimum",
+                "count": 1,
+                "labels": "release-notes:not-required, category:internal",
+                "message": dedent(
+                    """
+                    Please do one of:
+
+                    - add release notes to the appropriate file in `docs/notes`
+
+                    - label this PR with `release-notes:not-required` if it does not need them (for
+                      instance, if this is fixing a minor typo in documentation)
+
+                    - label this PR with `category:internal` if it's an internal change
+
+                    Feel free to ask a maintainer for help if you are not sure what is appropriate!
+                    """
+                ),
+            },
+        },
+    ]
+
+
 def checkout(
     *,
     fetch_depth: int = 10,
@@ -957,6 +993,13 @@ def test_workflow_jobs() -> Jobs:
             "if": IS_PANTS_OWNER,
             "steps": ensure_category_label(),
         },
+        "check_release_notes": {
+            "name": "Ensure PR has release notes",
+            "runs-on": linux_x86_64_helper.runs_on(),
+            "needs": ["classify_changes"],
+            "if": IS_PANTS_OWNER,
+            "steps": ensure_release_notes(),
+        },
     }
     jobs.update(**linux_x86_64_test_jobs())
     jobs.update(**linux_arm64_test_jobs())
@@ -1643,7 +1686,7 @@ def merge_ok(pr_jobs: list[str]) -> Jobs:
             # NB: This always() condition is critical, as it ensures that this job is run even if
             #   jobs it depends on are skipped.
             "if": "always() && !contains(needs.*.result, 'failure') && !contains(needs.*.result, 'cancelled')",
-            "needs": ["classify_changes", "check_labels"] + sorted(pr_jobs),
+            "needs": ["classify_changes", "check_labels", "check_release_notes"] + sorted(pr_jobs),
             "outputs": {"merge_ok": f"{gha_expr('steps.set_merge_ok.outputs.merge_ok')}"},
             "steps": [
                 {
@@ -1685,7 +1728,7 @@ def generate() -> dict[Path, str]:
     pr_jobs = test_workflow_jobs()
     pr_jobs.update(**classify_changes())
     for key, val in pr_jobs.items():
-        if key in {"check_labels", "classify_changes"}:
+        if key in {"check_labels", "classify_changes", "check_release_notes"}:
             continue
         needs = val.get("needs", [])
         if isinstance(needs, str):