chore: add PR template and required CI checks (V2-719, V2-720) - #160
Conversation
Add the standard pull request template (V2-719) and the CI enforcement that makes a linked Linear issue and the template fields required on every PR (V2-720). Part of the release-process extensions; identical across the six crate repos. - .github/PULL_REQUEST_TEMPLATE.md: eight-field template with Risk tier and Semver impact checkboxes, stable headings as grep anchors for the train-manifest verification. - CLAUDE.md: instruct agents to fill the template on PR open. - .github/workflows/pr-checks.yml + .github/scripts/check_pr.py: the linear-link (main + rc-*) and pr-template (main) required status checks. V2-719 V2-720 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
dirvine
left a comment
There was a problem hiding this comment.
I found two gaps in the enforcement, reproduced against the checker on this PR head:
-
The Linear gate does not reliably require a Linear issue.
LINEAR_KEYaccepts any<letters>-<digits>token anywhere in the title, body, or branch. For example, a PR containing onlyUTF-8as its supposed issue reference passes bothlinearandtemplate. Common technical strings such asSHA-256,HTTP-2, orRFC-123can therefore satisfy the required check accidentally. Please require a reallinear.app/.../issue/...URL, or constrain keys to the organisation's known Linear team prefixes. -
“Fill every field” is not enforced. A Compatibility section with only
Wire: nonepasses whileStorage:andAPI:are blank, and a T0/T1 PR also passes with an empty ADR section. I reproduced that combination locally and receivedPR template complete. Please require all three compatibility axes and require an explicit ADR value (n/afor T0/T1; a link for T2/T3).
The six PRs currently carry the same checker (SHA-256 89aa9a45c0444cb40726a3e5d3b8cc5e402d75ed4e50341fb2fff84053098649), so the same fix should be applied consistently across them. The template/workflow shape otherwise looks sound, and the new checks themselves are running successfully.
Address review feedback on the release-process PR checks: - Linear gate: require a real Linear reference — a linear.app URL or an issue key with a known team prefix (V2/AUTO/REL/INFRA/QA) — so tokens like UTF-8, SHA-256, RFC-123 no longer satisfy it. Strip HTML comments before matching so the template's own V2-123 example does not count. - Template completeness: require all three Compatibility axes (Wire/Storage/API) and an explicit ADR value (n/a for T0/T1, a link for T2/T3); previously one filled axis and an empty ADR passed. - Fix a regex that let an empty Compatibility axis match the next line. - Emit exact template headings in "missing section" errors. - Run the workflow on ready_for_review too. V2-719 V2-720 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thanks — both gaps are fixed, and the same checker (identical across all six PRs) is updated consistently. 1. Linear gate no longer accepts arbitrary tokens. A reference now counts only if it is a 2. "Fill every field" is enforced. All three Compatibility axes (Wire/Storage/API) must each carry a value ( Also addressed from the Copilot review: exact template headings in the "missing section" messages ( One point I did not change yet: Copilot's tamper-resistance note (switch |
dirvine
left a comment
There was a problem hiding this comment.
The compatibility and ADR enforcement is fixed, and the constrained issue-key prefixes correctly reject UTF-8/SHA-256-style tokens. One residual gap remains in the Linear URL path:
LINEAR_URL = re.compile(r"linear\.app/", re.IGNORECASE) accepts any page on the domain. I reproduced both https://linear.app/changelog and https://linear.app/not-an-issue passing the linear and template checks as a linked issue.
Please constrain the URL pattern to an actual issue path, for example linear.app/<workspace>/issue/<key> (allowing the optional issue slug if needed), and include this negative case in the test matrix. Everything else from my previous review is addressed.
Follow-up review: the linear.app URL check accepted any page on the domain (linear.app/changelog, linear.app/not-an-issue passed). Constrain it to a real issue path (linear.app/<workspace>/issue/<key>). - Linear URL must now match linear.app/<workspace>/issue/<key>[/<slug>]. - Clarify the failure message (a reference is accepted in title, body, or branch, matching the actual behaviour). - Add .github/scripts/test_check_pr.py: a 17-case matrix exercising the real checker, including the linear.app/changelog and linear.app/not-an-issue negatives; run it in CI via a new self-test job. V2-719 V2-720 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Fixed — and thanks for catching the loose URL path. Linear URL is now constrained to a real issue path. Both negatives are in a committed test matrix. Added The same checker + tests are identical across all six PRs ( |
dirvine
left a comment
There was a problem hiding this comment.
Follow-up verified. The Linear URL matcher now requires an actual <workspace>/issue/<key> path, the two reported false-positive URLs are covered by regression tests, and the shared test matrix passes 17/17 locally. The required linear-link and pr-template checks are green. Approved.
Linear issue
Risk tier
Compatibility
Semver impact
Test evidence
check_pr.pywas exercised locally across its cases: missing Linear referencefails
linear; a key in the title/body/branch passes; a body without thetemplate fails
template; an rc-* base is a no-op pass; a fully-filled templatepasses; and the failure modes (no/duplicate tier or semver box, empty section,
Tier 2/3 without an ADR link) each fail with a specific message. This PR's own
description is validated by the new checks.
New dependency
none
ADR
n/a
Mitigation / rollback
Revert this PR; the template and checks are additive and carry no runtime effect.