Skip to content

Improve changelog validations#7637

Merged
erosselli merged 6 commits intomainfrom
erosselli/changelog-validation-updates
Mar 13, 2026
Merged

Improve changelog validations#7637
erosselli merged 6 commits intomainfrom
erosselli/changelog-validation-updates

Conversation

@erosselli
Copy link
Contributor

Ticket []

Description Of Changes

Code Changes

Steps to Confirm

Pre-Merge Checklist

  • Issue requirements met
  • All CI pipelines succeeded
  • CHANGELOG.md updated
    • Add a db-migration This indicates that a change includes a database migration label to the entry if your change includes a DB migration
    • Add a high-risk This issue suggests changes that have a high-probability of breaking existing code label to the entry if your change includes a high-risk change (i.e. potential for performance impact or unexpected regression) that should be flagged
    • Updates unreleased work already in Changelog, no new entry necessary
  • UX feedback:
    • All UX related changes have been reviewed by a designer
    • No UX review needed
  • Followup issues:
    • Followup issues created
    • No followup issues
  • Database migrations:
    • Ensure that your downrev is up to date with the latest revision on main
    • Ensure that your downgrade() migration is correct and works
      • If a downgrade migration is not possible for this change, please call this out in the PR description!
    • No migrations
  • Documentation:
    • Documentation complete, PR opened in fidesdocs
    • Documentation issue created in fidesdocs
    • If there are any new client scopes created as part of the pull request, remember to update public-facing documentation that references our scope registry
    • No documentation updates required

@erosselli erosselli requested a review from johnewart March 12, 2026 16:43
@vercel
Copy link
Contributor

vercel bot commented Mar 12, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Actions Updated (UTC)
fides-plus-nightly Ignored Ignored Preview Mar 12, 2026 5:20pm
fides-privacy-center Ignored Ignored Mar 12, 2026 5:20pm

Request Review

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 12, 2026

Greptile Summary

This PR hardens changelog fragment validation by introducing several new checks in noxfiles/changelog_nox.py and wiring the GitHub PR number through the CI workflow so it can be cross-validated against fragment contents.

Key improvements:

  • Filename format enforcement – fragments must now follow <pr_number>-<slug>.yaml; non-conforming names are rejected via the new FILENAME_PATTERN regex.
  • PR number consistency – the numeric prefix in the filename must match the pr: field in the YAML, and both are now validated against the actual GitHub PR number supplied via --pr-number in CI.
  • Placeholder description detection – entries whose description still contains template boilerplate (e.g. "Short description of the change") are rejected.
  • Label type validationlabels: must be a YAML list (addresses previous thread concern about scalar strings iterating character-by-character).
  • Trailing whitespace stripping – descriptions are .strip()'ed before storage and markdown rendering.
  • Refactoring_load_and_validate_file() and _find_unreleased_section_with_version() were extracted to eliminate two pairs of identical code blocks.

One issue found: data["pr"] is read directly from YAML and compared to the integer extracted from the filename without first asserting it is an int. If a contributor writes pr: "7637" (quoted), the comparison produces a confusing error showing two visually identical numbers as a "mismatch".

Confidence Score: 4/5

  • Safe to merge with a minor fix recommended — the missing integer type check on data["pr"] can produce confusing error messages but does not break valid changelog entries.
  • All four files are purely tooling/CI changes with no production code impact. The new validations are well-structured and address several real usability issues. The one identified bug (missing type check on the pr YAML field) only affects the error message quality for malformed inputs, not the happy path, so it does not block merging.
  • noxfiles/changelog_nox.py — the validate_fragment_data function needs an isinstance(data["pr"], int) guard before the filename/PR comparison.

Important Files Changed

Filename Overview
noxfiles/changelog_nox.py Major improvements: added placeholder description detection, filename format validation (FILENAME_PATTERN), label type checking (addresses prior thread concern), and PR number cross-validation. Extracted _load_and_validate_file() and _find_unreleased_section_with_version() to eliminate two identical code blocks. One issue: data["pr"] is never validated as an integer type before comparing with filename_pr, so pr: "7637" (quoted string in YAML) produces a confusing "mismatch" error showing identical numbers. The removed if label in LABEL_URLS: guard in to_markdown() is safe given upstream validation but reduces robustness of the standalone method.
.github/workflows/check_changelog.yml Passes pr.number as a new pr_number output and threads it into the validation step via --pr-number. The if: success() guard on the validate step means it only runs when the PR number is set, so no empty-string edge case. Change is minimal and correct.
changelog/TEMPLATE.yaml Updated the comment to prescribe the new required filename format <pr_number>-<short-description>.yaml, consistent with the new FILENAME_PATTERN validation.
changelog/7637-improve-changelog-validations.yaml New changelog entry for this PR; filename, type, description, and pr fields are all valid and consistent.

Last reviewed commit: 59fe9f6

erosselli and others added 2 commits March 12, 2026 13:47
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@erosselli
Copy link
Contributor Author

@greptile re-review

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@erosselli erosselli added this pull request to the merge queue Mar 13, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 13, 2026
@erosselli erosselli added this pull request to the merge queue Mar 13, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 13, 2026
@erosselli erosselli added this pull request to the merge queue Mar 13, 2026
Merged via the queue into main with commit 672b8b5 Mar 13, 2026
57 checks passed
@erosselli erosselli deleted the erosselli/changelog-validation-updates branch March 13, 2026 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants