Skip to content

Add CHANGELOG.md modification check and warning comment - #4217

Open
mario-campos wants to merge 13 commits into
mainfrom
mario-campos/warn-on-changelog-write
Open

mario-campos wants to merge 13 commits into
mainfrom
mario-campos/warn-on-changelog-write

Conversation

@mario-campos

@mario-campos mario-campos commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

This PR adds a PR check for non-release/mergeback/update-bundle PRs. The check posts a sticky PR comment if CHANGELOG.md has been modified in the PR, warning the author to instead direct their change to a change-note.

Risk assessment

For internal use only. Please select the risk level of this change:

  • Low risk: Changes are fully under feature flags, or have been fully tested and validated in pre-production environments and are highly observable, or are documentation or test only.

Which use cases does this change impact?

Workflow types:

  • N/A

Products:

  • N/A

Environments:

  • Testing/None - This change does not impact any CodeQL workflows in production.

How did/will you validate this change?

  • None - I am not validating these changes.

If something goes wrong after this change is released, what are the mitigation and rollback strategies?

  • Development/testing only - This change cannot cause any failures in production.

How will you know if something goes wrong after this change is released?

  • N/A

Are there any special considerations for merging or releasing this change?

  • No special considerations - This change can be merged at any time.

Merge / deployment checklist

  • Confirm this change is backwards compatible with existing workflows.
  • Consider adding a changelog entry for this change.
  • Confirm the readme and docs have been updated if necessary.

@github-actions github-actions Bot added the size/XS Should be very easy to review label Oct 8, 2026

@mbg mbg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Couple of initial comments on this draft. I like that this is taking advantage of the setup that the existing fetch-base provides.

I haven't checked if the changelog-check step behaves as expected yet either. One thing I did note though related to it is that the fetch-base step is skipped if github.event.pull_request.user.login != 'dependabot[bot]'. That isn't new in this PR (and we probably shouldn't address it here), but it might be better to check github.triggering_actor instead of github.event.pull_request.user.login. The justification in the comment is that Dependabot doesn't have permission, but it is possible for us to push commits to Dependabot PR branches or re-run workflows on it.

Also agree that it would be good to not spam the comment and instead replace an existing one. It's fairly easy to do that with gh or in a script by leaving a comment (<!-- some distinct marker -->) in the comment that's posted and then looking for that.

Comment thread .github/workflows/pr-checks.yml Outdated
Comment thread .github/workflows/pr-checks.yml Outdated
@github-actions github-actions Bot added size/S Should be easy to review and removed size/XS Should be very easy to review labels Oct 8, 2026
@mario-campos
mario-campos marked this pull request as ready for review October 8, 2026 17:43
@mario-campos
mario-campos requested a review from a team as a code owner October 8, 2026 17:43
Copilot AI balanced review requested due to automatic review settings October 8, 2026 17:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Shell command substitution currently prevents warning creation, and comment lifecycle and exclusion logic need correction.

3 open findings
What changed in this PR

Adds a PR check that warns contributors to use change notes instead of editing CHANGELOG.md directly.

Changes:

  • Detects CHANGELOG.md modifications.
  • Adds a sticky warning comment with automation exclusions.
  • Consolidates PR comment posting into one job.
File Description
.github/​workflows/​pr-checks.yml Detects changelog edits and posts PR warnings.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .github/workflows/pr-checks.yml Outdated
Comment thread .github/workflows/pr-checks.yml Outdated
Comment thread .github/workflows/pr-checks.yml Outdated
mario-campos and others added 2 commits October 8, 2026 12:55
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Refactor conditions for PR title checks in workflow.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@mario-campos

Copy link
Copy Markdown
Contributor Author

@copilot Are there unresolved findings?

Copilot AI commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

@copilot Are there unresolved findings?

Yes—one valid finding remains: the warning step is gated on changelog-changed == 'true', so if a later commit reverts the CHANGELOG.md change, the step never runs and the existing warning comment stays stale. It should also run when the file is unchanged and remove any existing marker comment. The shell backticks are escaped, and the automation exclusion checks now include branch criteria.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The workflow correctly handles comment creation, deduplication, cleanup, and automation exemptions.

0 open findings

3 resolved since last review

🧠 Review effort: Balanced

@mbg mbg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for changing this to move away from pulling in another action and also moving the step that posts or updates the comment out of other-checks and into (now) post-pr-comments.

I added a few more comments, but the main one is about avoiding more, lengthy-ish shell scripts. It shouldn't be hard to implement in TS in pr-checks and I have included some relevant pointers in my detailed comment about this.

Comment thread .github/workflows/pr-checks.yml Outdated
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
PR_NUMBER: ${{ github.event.pull_request.number }}
CHANGED: ${{ needs.other-checks.outputs.changelog-changed }}
run: |

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I am not keen on adding more shell scripts in workflow steps beyond very short ones, and this one is now a bit long with the $CHANGED check added as well.

Can you turn this into a new script in pr-checks instead? We already have a couple of helpers in api-client.ts there. See sync-checks.ts or update-release-branch.ts for examples of scripts that use the API there.

The script doesn't even have to be all that specific to this. The "create-or-update-comment" pattern is probably useful in a few different places.

Comment thread .github/workflows/pr-checks.yml Outdated

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/S Should be easy to review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants