Skip to content

ci: PR template and stricter danger.js review checks#1202

Merged
alandefreitas merged 3 commits into
cppalliance:developfrom
alandefreitas:develop
May 13, 2026
Merged

ci: PR template and stricter danger.js review checks#1202
alandefreitas merged 3 commits into
cppalliance:developfrom
alandefreitas:develop

Conversation

@alandefreitas
Copy link
Copy Markdown
Collaborator

Part of our ongoing work to make review processes more efficient now that AI-assisted contributions routinely produce large diffs. Inspired by LLVM's AI Tool Use Policy, this PR includes a few improvements to the automated review workflow. Existing danger.js coverage was per-commit and used a fixed 40-character description floor, which let well-sliced multi-thousand-line PRs with two-sentence bodies land without any automated signal. Three changes here address that:

  1. PR template so contributors are prompted to describe the change in a structured way (and satisfy the danger.js checks naturally when filled in).
  2. Two new danger.js rules covering what the existing per-commit and 40-char-floor rules missed: an aggregate source-churn warning (5000 lines), and a log-scaled description floor so descriptions are expected to grow with the size of the change.
  3. (incidental) .gitignore entries for Python coverage artifacts that bootstrap.py and util/bootstrap/ leave behind.

Changes

  • Tests: Unit tests for the new danger.js rules (detail under Testing).
  • CI: New .github/pull_request_template.md; two new rules in util/danger/logic.ts (aggregateSizeWarnings, expectedBodyLength); cleanBody() strips HTML comments before measuring length so template scaffolding cannot inflate the count; util/danger/README.md updated to describe the new rules; .gitignore extended with .coverage, .coverage.*, and htmlcov/.
  • Breaking changes: None.

Testing

  • util/danger/logic.test.ts adds unit tests for aggregateSizeWarnings, expectedBodyLength, the body-length cascade (empty / short-for-size / well-matched), cleanBody() HTML-comment stripping, and an end-to-end case through evaluateDanger exercising a large multi-file PR shape.
  • No CI workflow changes needed: the existing Repo checks job already runs npm test under util/danger/ and executes the dangerfile on every PR, so the new rules and their tests stay covered on every future build.

Documentation

util/danger/README.md updated to describe the two new rules and to point at the new PR template. No user-facing documentation changes — these are repository-infrastructure improvements.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 13, 2026

🧾 Changes by Scope

Scope Lines Δ% Lines Δ Lines + Lines - Files Δ Files + Files ~ Files ↔ Files -
⚙️ CI 100% 273 271 2 5 1 4 - -
Total 100% 273 271 2 5 1 4 - -

Legend: Files + (added), Files ~ (modified), Files ↔ (renamed), Files - (removed)

🔝 Top Files

  • util/danger/logic.test.ts (CI): 170 lines Δ (+170 / -0)
  • util/danger/logic.ts (CI): 71 lines Δ (+69 / -2)
  • .github/pull_request_template.md (CI): 25 lines Δ (+25 / -0)

Generated by 🚫 dangerJS against e77f908

@codecov
Copy link
Copy Markdown

codecov Bot commented May 13, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.12%. Comparing base (1fc06a0) to head (e77f908).

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #1202   +/-   ##
========================================
  Coverage    82.12%   82.12%           
========================================
  Files           33       33           
  Lines         3149     3149           
  Branches       734      734           
========================================
  Hits          2586     2586           
  Misses         387      387           
  Partials       176      176           
Flag Coverage Δ
bootstrap 82.12% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f884b346eb

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread util/danger/logic.ts Outdated
@cppalliance-bot
Copy link
Copy Markdown

cppalliance-bot commented May 13, 2026

An automated preview of the documentation is available at https://1202.mrdocs.prtest2.cppalliance.org/index.html

If more commits are pushed to the pull request, the docs will rebuild at the same URL.

2026-05-13 21:44:03 UTC

Adds an aggregate source-churn warning (5000 lines) so large PRs are
flagged even when individual commits stay under the per-commit limit,
and replaces the fixed 40-character description floor with a log-scaled
threshold (~80 * log2(churn)) so descriptions are expected to grow with
the size of the change. Template scaffolding (HTML comments, Markdown
headers, standalone italic placeholders, and `- **Label**: _placeholder_`
bullets) is stripped from the body before measurement so an unfilled
template cannot inflate the length count or mask the testing-mention
check via the `## Testing` header.
Prompts contributors to describe the change, enumerate per-area
impacts, document testing (including any workflow updates), and note
documentation changes. Placeholders are visible italic text so any
section a contributor forgets to replace stands out in the rendered PR.
@alandefreitas alandefreitas merged commit ab5f412 into cppalliance:develop May 13, 2026
25 checks passed
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