Repository navigation
Scope the No-Check-Posted Cause to DIRTY and Give It a Remedy - #3490
Conversation
The third promotion review of #3484 found the exit 45 entry and README row offering "no check posted" for any gated merge, though only DIRTY releases one, and that cause naming no remedy. It now says to resolve the head's conflict with its base, and the test table checks a reason only where one is expected. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The local pass found the exit 0 paragraph still offering exit 45 for a rollup with none posted under any gated merge, and no test pinning the new remedy. The paragraph now scopes that clause to DIRTY, and the test asserts the remedy. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
A recorded local strict-review pass covers head |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
 |
There was a problem hiding this comment.
🔵 Needs a closer look
The conflict-remedy test should assert the complete “with its base” wording promised by the implementation.
No open issues 🎉
What changed in this PR
This pull request scopes the no-check-posted condition to DIRTY merges, documents the conflict-resolution remedy, and strengthens related test assertions without changing behavior or exit codes.
Changes:
- Scope exit
45documentation toDIRTYmerges. - Explain that the head’s conflict with its base must be resolved.
- Replace a vacuous empty-reason assertion with conditional checks.
| File | Summary |
|---|---|
tests/test_pr_review.py |
Updates conflict-remedy and reason assertions. |
scripts/README.md |
Clarifies the narrowed exit-45 condition. |
scripts/pr_review.py |
Updates wait help text for the scoped behavior. |
scripts/pr_review_checks.py |
Adds conflict-resolution guidance to the diagnostic. |
Review effort: Lite
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #3490 +/- ##
========================================
Coverage 81.04% 81.04%
========================================
Files 25 25
Lines 8922 8922
========================================
Hits 7231 7231
Misses 1691 1691
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Answering the round's headline, "The conflict-remedy test should assert the complete 'with its base' wording promised by the implementation": declined as a preference between defensible forms. The assertion in test_an_empty_rollup_holds_any_merge_but_a_conflicted_one pins the remedy itself, "but no check has posted, so resolve the head's conflict", which is what a regression dropping the remedy would lose, and fleet-code-review's Review the Change asks reviewers to omit preferences. |
) ## Summary Promotes the pull requests merged into develop since release 2.0.1184. - [#3483](#3483): a held `wait` exits `45` with `status=CHECKS_MISSING` where it used to exit `0` over checks that never ran. The cases are a payload with no commit matching the head, a `DIRTY` merge with no check posted, and a `DIRTY` or `BEHIND` merge whose required context is unregistered or came from another app. The `wait` help and `scripts/README.md` name each exception that stays at `0`. - [#3485](#3485): the exit-0 list in the help and README names a pull request no longer open, which this promotion's own review found missing. - [#3486](#3486): where the head's rollup holds a type the reader cannot parse, the `CHECKS_MISSING` clause and the `BLOCKED` hold's note say a required check may sit in that unread part, which this promotion's second review found missing. - [#3490](#3490): the no-check-posted cause of exit `45` is scoped to `DIRTY` in the help and README and names its remedy, which this promotion's third review found missing. - [#3491](#3491): the `wait` help and README restore the caveat that a review-present exit `0` does not poll the checks, which this promotion's fourth review found dropped. Closes #3471 ## Constituent coverage Each change this promotion carries had coverage on its own pull request into develop: | Pull request | Merged head | Coverage | | --- | --- | --- | | [#3483](#3483) | `8f29cadd` | attested local pass at the merged head, base `develop`. A Copilot round on the earlier head `ee530a4b` opened three findings, each fixed in `995ce78a` and resolved, so no Copilot round covers the merged head | | [#3485](#3485) | `275e4c8b` | attested local pass at the merged head, base `develop`, and a Copilot table on the merged head naming exactly its 2 changed files, no open findings | | [#3486](#3486) | `1f9fde03` | attested local pass at the merged head, base `develop`. A Copilot round on the earlier head `b8a74e8a` opened three findings, each fixed later on the branch and resolved, so no Copilot round covers the merged head | | [#3490](#3490) | `ba37c856` | attested local pass at the merged head, base `develop`, and a Copilot table on the merged head naming exactly its 4 changed files. Its headline's test-wording preference was declined in a comment | | [#3491](#3491) | `8c65e4d8` | attested local pass at the merged head, base `develop`, and a Copilot table on the merged head naming exactly its 2 changed files, verdict clean, no open findings | 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Summary
The third promotion review of #3484 raised three findings, each fixed here:
waithelp's exit 0 paragraph and45entry, and the README's45row, offered "no check posted" for any merge other thanCLEAN,UNSTABLE, orHAS_HOOKS. The empty-rollup hold keepsBLOCKED,BEHIND, andDRAFTat30, so each now scopes that cause toDIRTY.assertIn("", ...)on its exit 0 rows, which always passes. It now checks a reason only where one is expected, and those rows stay covered by theirCHECKS_MISSINGabsence check.Behavior and exit codes are unchanged.
Part of #3471, which the promotion pull request closes.
Test plan
python -m unittest test_pr_review test_pr_review_size test_pr_review_copilotfromtests/: 534 tests pass.prose_lint.py --check sentence-lengthis clean over the diff.DIRTYalone and found no finding.🤖 Generated with Claude Code