Print the Pin-Resolution Note on Every Run, Including the Empty One - #610
Merged
Conversation
Raised on the promotion pull request. The docstring says the counts print on every run "so that narrowness is visible rather than inferred from a clean line", and the code guarded the note on a non-zero counter, so it went silent on exactly the run it exists for: one that resolved nothing, which is what a repository carrying no workflow at all produces. The comment inside the guard said "one fixed shape every run" while sitting behind a condition that made it conditional. The note is unconditional now, and a case covers the all-zero run rather than only the zeroes beside a count. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Makes check_sha_pin consistently emit its pin-resolution note even when all counters are zero, and adds a regression test that exercises the previously-uncovered all-zero run (e.g., a tree with no workflow files).
Changes:
- Remove the conditional guard so the pin-resolution note is appended on every
check_sha_pininvocation. - Add a unit test covering the “no workflows / all counters zero” case to prevent regressions.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| scripts/repo_gate.py | Always appends the pin-resolution note (including for all-zero runs). |
| scripts/test_repo_gate.py | Adds a regression test asserting the note is present when every count is zero. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Copilot asked for a shorter, present-tense rationale. The clarity half is taken: "the one reading it exists to surface" was an unclear phrase and the historical framing added nothing a reader needs. The two lines stay rather than becoming one. CODESTYLE.md licenses a multi-line inline comment for coupling a future edit could easily break, and re-adding the guard as an obvious optimization is exactly that edit, so the line saying what the guard would hide is the one worth keeping. Co-Authored-By: Claude Opus 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.
Raised by Copilot on the promotion pull request #609, against code that promotion carries.
The defect
check_sha_pin's docstring states the reason the note exists:The code guarded it on a non-zero counter, so it printed on every run except the one it was written for. A repository carrying no workflow file at all resolves nothing, every counter is zero, and the note vanished, leaving precisely the clean line the sentence says nobody should have to infer the check's narrowness from.
The comment inside the guard made the contradiction plain, reading "One fixed shape every run, so a zero in any position is as visible as a count" while sitting behind the condition that made it conditional.
Reproduced before the fix, against a directory with no workflows:
The fix
The note is unconditional. After it, the same call reports what it did rather than nothing:
The existing case,
test_the_note_carries_every_count_including_the_zeroes, asserted the zeroes only where a count sat beside them, which is why the all-zero run went uncovered. A case now drives that run directly.repo_gate.pyon this repository is unchanged in substance and still reports45 under another owner, which is the number that made the hub's own zero worth surfacing in the first place.🤖 Generated with Claude Code