Skip to content

gha: enforce CHANGELOG.md entries for PRs #5047

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.

Sign up for GitHub

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

Closed
cyphar wants to merge 1 commit into from
Closed

gha: enforce CHANGELOG.md entries for PRs #5047

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions .github/workflows/validate.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,34 @@ jobs:
if: github.event_name != 'pull_request'
run: echo "Nothing to check here."

changelog:
runs-on: ubuntu-24.04
steps:
- uses: actions/checkout@v6
if: github.event_name == 'pull_request'
with:
fetch-depth: 0
- name: git fetch ${{ github.ref }}
if: github.event_name == 'pull_request'
run: |-
git fetch origin ${{ github.ref }}
git tag GITHUB_REF FETCH_HEAD
- name: git fetch ${{ github.base_ref }}
if: github.event_name == 'pull_request'
run: |-
git fetch origin ${{ github.base_ref }}
git tag GITHUB_BASE_REF FETCH_HEAD
- name: check that CHANGELOG.md has an entry
if: github.event_name == 'pull_request'
run: >-
git diff --name-only GITHUB_BASE_REF...GITHUB_REF \
| grep CHANGELOG.md \
|| { echo "Every PR must have a CHANGELOG.md change."; exit 1; }
Copy link
Copy Markdown
Member

@AkihiroSuda AkihiroSuda Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dependabot PRs should be excluded?
PRs with some directive like [trivial] in title or /trivial in body should be excluded too?

Copy link
Copy Markdown
Member

@AkihiroSuda AkihiroSuda Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe reusable:
https://github.com/moby/moby/blob/1da6517e1a4381297e56862f6f373f265c28d102/.github/workflows/validate-pr.yml#L34-L70


- name: succeed (not a PR) # Allow all-done to succeed for non-PRs.
if: github.event_name != 'pull_request'
run: echo "Nothing to check here."

cfmt:
runs-on: ubuntu-24.04
steps:
Expand Down Expand Up @@ -267,6 +295,7 @@ jobs:
needs:
- check-go
- cfmt
- changelog
- codespell
- commit
- compile-buildtags
Expand Down
5 changes: 5 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
patched kernels (such as the Tencent kernels). (opencontainers/cgroups#46,
opencontainers/cgroups#50)

### Changed ###
- CI: All PRs now require a corresponding `CHANGELOG.md` change be included,
Copy link
Copy Markdown
Member

@AkihiroSuda AkihiroSuda Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is likely to just result in making the changelog lengthy and unreadable.
Also we will no longer be able to merge dependabot PRs.

Copy link
Copy Markdown
Member

@AkihiroSuda AkihiroSuda Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe just migrate the changelog to wiki so that we can add changelog items without submitting PRs

Copy link
Copy Markdown
Member Author

@cyphar cyphar Nov 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How will we make sure it gets updated on time? The current situation is that PRs get merged and then you need to figure out what to write for the changelog months later.

I would prefer to not have it be mandatory but it seems we are nowhere near consistent enough to enforce this through regular reviews.

For dependabot or minor PRs, I can adjust this CI job to allow-list PRs with special tags or by special committer emails...

One other option would be to enforce having a "for the changelog" section of the PR but then we would probably want to script collating them...

Copy link
Copy Markdown
Contributor

@kolyshkin kolyshkin Dec 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree we have to add this somehow, and I guess adding the entry directly to CHANGELOG.md seems most straightforward. The downsides are

  • there will definitely be more merge conflicts, especially for backports;
  • dependabot PRs won't work (guess we can add an exception);
  • the changelog style might end up less uniform;
  • might make things more complicated for new contributors.

Perhaps we can try to enforce this manually for now?

Copy link
Copy Markdown
Member Author

@cyphar cyphar Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've seen other projects avoid the merge conflict problem by using a directory and making a new file for each change. This has some extra overhead for everyone though, as the filenames are mean to be the PR number (submitters need to re-commit with the right PR number and we will still have to collate them).

For the dependabot PR issue we can make it a non-required CI job and so we can merge dependabot PRs but human PRs will have a failure so the submitter can proactively fix the issue.

Perhaps we can try to enforce this manually for now?

I think it'd be too easy to forget and having it be a CI job means that submitters can proactively fix it without needing us to remember to tell them.

which should increase the quality and accuracy of our changelogs going
forward. (#5047)

## [1.4.0-rc.1] - 2025-09-05

> おめェもボスになったんだろぉ？
Expand Down