Repository navigation
fix(review): PR checks are not retroactive and attest destructive sites per commit (unblocks #1437) - #1567
Merged
Conversation
… sites per commit (Spec 11.06 sections 3.1, 3.2) The release PR for the 2.11.0 line (#1437, base develop) failed the new Review gates check: its range holds 139 commits merged before the check existed, so it re-judged 6 old destructive lines and demanded one trailer covering all 35 sites in the line. - When the commit that introduced scripts/review_gates.py lies inside merge-base..head, checking starts at that commit, so nothing reviewed before the checks existed is re-judged, and every change from the introducing commit onward still is. - Attestation is per commit: each commit that adds sites carries its own Destructive-Op-Check trailer for its own count (several trailers in one message, as a squash merge collects them, are summed). A single cumulative trailer cannot work for a release branch whose range holds many already-attested commits. - Warning comments are still checked on the head version of every added line. Tests: 5 new (per-commit attestation, missing per-commit trailer, squash summing, pre-check changes ignored, changes from the introducing commit checked). The #1437 range (3c0759b..3fcf20d) now reports 0 findings; the same range without the introducing commit still reports the 6 unwarned sites. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…t data is exempt (Spec 11.06 section 3.2) A squash merge concatenates the trailers of the PR's commits, each already checked exactly in that PR; when the PR reworked a destructive line their sum exceeds the squashed diff's net count. For a commit whose subject ends in (#N) the sum may exceed, never fall short of, its sites; every other commit still attests exactly. tests_e2e/ joins the test directories. 4 new tests (91 total, 4 skipped). Co-A 8C3D uthored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the failing Review check on #1437.
#1437 compares the whole 2.11.0 line with develop. Once #1565 merged, the new check ran over that range: 139 commits reviewed and merged before the check existed. It flagged 6 old destructive lines and demanded a single trailer covering all 35 destructive sites in the line, a count no commit could carry.
Changes to the check script and Spec 11.06:
Ranges checked with the new script:
Whole-Change-Verification: ran=python3 -m unittest discover -s scripts/tests (91 tests, 9 new, 4 skipped, all pass), python3 scripts/validate_specs.py --changed-base on 3fcf20d (84 specs, 1243 citations, PASSED), and the check script on the three ranges above; result=pass
Created on behalf of the team that co-maintains this connector; maintainers own the merge.
Generated with Claude Code (https://claude.com/claude-code)