Repository navigation
#2647 Add changelog entry check #2649
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
72d8de8
7b9db01
0a9bcc8
083b0fc
675bcc3
972e5cb
f47e254
f12e761
811347d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,21 +1,26 @@ | ||
| name: Check PR Labels | ||
| name: Check PR Labels & Changelog | ||
|
|
||
| # Fails (red check) while a pull request has no labels and turns green as soon as at | ||
| # least one label is assigned. It runs on the events that can change a PR's labels or | ||
| # its head commit (synchronize = author pushed new commits), so the check re-evaluates | ||
| # immediately and is never missing on a freshly pushed commit. No write access: this | ||
| # only reads the PR and reports a failure, so pull_request (not pull_request_target) | ||
| # is the correct trigger. | ||
| # - Fails while a pull request has no labels. | ||
| # - Passes as soon as at least one label is assigned. | ||
| # - Enforces the changelog policy: | ||
| # - "internal" PR: must not add a CHANGELOG.adoc entry. | ||
| # - Non-"internal" PR: must add a CHANGELOG.adoc entry. | ||
| # - Skips the changelog check until the PR has at least one label. | ||
| # - Re-runs when: | ||
| # - PR labels change. | ||
| # - New commits are pushed (synchronize). | ||
| # - Based on documentation/contributing/DoD.adoc: the changelog should contain only | ||
| # public, end-user-relevant changes. | ||
|
|
||
| on: | ||
| pull_request: | ||
| types: [opened, reopened, synchronize, labeled, unlabeled] | ||
| types: [ opened, reopened, synchronize, labeled, unlabeled ] | ||
|
|
||
| permissions: | ||
| pull-requests: read | ||
|
|
||
| jobs: | ||
| check-labels: | ||
| check-labels-and-changelog: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Verify the PR has at least one label | ||
|
|
@@ -38,3 +43,53 @@ jobs: | |
| + '(e.g. "bug", "enhancement", "documentation") to make this check pass.' | ||
| ); | ||
| } | ||
|
|
||
| - name: Verify internal label and CHANGELOG.adoc entry are mutually exclusive | ||
| # Only meaningful once the PR has labels; an unlabeled PR already failed the | ||
| # step above and has to add a label first. | ||
| if: success() | ||
| uses: actions/github-script@v7 | ||
| with: | ||
| script: | | ||
| const INTERNAL_LABEL = 'internal'; | ||
| const CHANGELOG = 'CHANGELOG.adoc'; | ||
| const prNumber = context.payload.pull_request.number; | ||
|
|
||
| // Fetch the PR fresh so the label state is authoritative for this run. | ||
| const pr = await github.rest.pulls.get({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| pull_number: prNumber | ||
| }); | ||
|
|
||
| const labels = pr.data.labels.map(label => label.name); | ||
| const isInternal = labels.includes(INTERNAL_LABEL); | ||
|
|
||
| // Paginate in case a PR changes more than 100 files. "additions" counts | ||
| // new lines in that file. | ||
| const files = await github.paginate( | ||
| github.rest.pulls.listFiles, | ||
| { owner: context.repo.owner, repo: context.repo.repo, pull_number: prNumber, per_page: 100 } | ||
| ); | ||
| const changelog = files.find(file => file.filename === CHANGELOG); | ||
| const addedLines = changelog ? changelog.additions : 0; | ||
|
|
||
| if (isInternal && addedLines > 0) { | ||
| core.setFailed( | ||
| `This PR has the "${INTERNAL_LABEL}" label but adds ${addedLines} line(s) to ${CHANGELOG}. ` | ||
| + `Internal changes have no user-facing impact and should not be documented. ` | ||
| + `Please remove the entry from ${CHANGELOG}, or remove the "${INTERNAL_LABEL}" ` | ||
| + `label if the change is user-facing. See documentation/contributing/DoD.adoc.` | ||
| ); | ||
| } else if (!isInternal && addedLines === 0) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Dependabot PRs will always fail here, since they only get labels:
- "dependencies"
- "internal"
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could do that, but maybe before we make these workflow changes we should get @hohwille 's opinion. |
||
| core.setFailed( | ||
| `This PR is not labelled "${INTERNAL_LABEL}" but adds no line to ${CHANGELOG}. ` | ||
| + `Please document this user-facing change by adding a line to ${CHANGELOG} under the next ` | ||
| + `open release, or add the "${INTERNAL_LABEL}" label if the change has no user-facing ` | ||
| + `impact. See documentation/contributing/DoD.adoc.` | ||
| ); | ||
| } else { | ||
| console.log(isInternal | ||
| ? `PR is labelled "${INTERNAL_LABEL}" and adds no line to ${CHANGELOG}.` | ||
| : `${CHANGELOG} has ${addedLines} added line(s).`); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This branch fails for PRs that the DoD explicitly exempts from the CHANGELOG, namely follow-up fixes to a story that has not been released yet (DoD.adoc, line 36). The same applies to test-only, workflow and docs PRs without the
internallabel: simulating this rule on the last 100 merged PRs, 17 non-Dependabot PRs would have failed (e.g. #2383, #2435, #2350, #2564). Could we update that DoD paragraph to say "label such PRsinternal", so contributors following the DoD do not end up with a red check?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aren't test-only and doc PRs internal changes? Probably we should update the DoD then.