chadtmiller.com/knotless
git that lives in your Atmosphere account
A gate's review shows as a required reviewer
Closed
This adds gate to reviewView. getThread sets it when the rule on the pull's base names the reviewer as a gate. The Reviewers menu then shows "The branch waits for its review" for a gate and keeps the old note for other non-writers. The lexicon, api.md and a test of the default (absent) case are updated with it. The lookup via record.rule(base) and the skip_serializing_if match the surrounding code.
I read the diff but didn't check the runner's results, so I haven't confirmed the 100% coverage gate. The test only covers a pull with no gate, so the any(|g| g.did == r.by) closure may never run in the Rust tests. If just cov flags it, add a gate case to the test.
The new sentence in the ReviewersFact.svelte doc comment is wrapped onto one very long line. This is cosmetic.
The new commit changes the gate's note in the menu from "The branch waits for its review" to "Required reviewer". The gate field, the getThread lookup, the lexicon and api.md are unchanged since my last review. The checks are still queued, so I haven't seen them pass. The Rust test still only covers the absent-gate case. If just cov flags the any(|g| g.did == r.by) closure, add a case with a gate.
The reworded doc comment in ReviewersFact.svelte now reads badly, and the very long line from last time is still there. "A gate the branch waits for is a required reviewer, and another who may not write, that their approval does not count" is not a sentence. This is cosmetic, so it doesn't block the merge.
web/src/lib/molecules/ReviewersFact.sveltelines 10 to 11
* have not reviewed it yet. A gate the branch waits for is a required reviewer, and | |
* another who may not write, that their approval does not count. With `people` and `ask`, as its author, it |
This sentence is broken: "and another who may not write, that their approval does not count". It also runs long. Something like: "A gate is marked as a required reviewer; another reviewer who may not write is marked as not counted."
This adds gate to reviewView. getThread sets it when the rule on the pull's base names the reviewer as a gate. The Reviewers menu then shows "Required reviewer" for a gate. The new commit hoists the rule lookup so the gates are those of the pull's base. That is correct, and the lexicon and api.md already describe it.
I didn't check the runner's results, so I haven't confirmed the coverage gate. The Rust test still only covers the absent-gate case. If just cov flags the any(|g| g.did == r.by) closure, add a case with a gate.
My earlier comment on the ReviewersFact.svelte doc comment is still open. The sentence "and another who may not write, that their approval does not count" is still broken, and the lines are still wrapped oddly. This is cosmetic, so it doesn't block the merge, but it is worth fixing while you are there.
web/src/lib/molecules/ReviewersFact.sveltelines 10 to 11
* have not reviewed it yet. A gate the branch waits for is a required reviewer, and | |
* another who may not write, that their approval does not count. With `people` and `ask`, as its author, it |
Still not a sentence. Something like: "A gate is marked as a required reviewer; another reviewer who may not write is marked as not counted."
Folded into Maintainers dismiss reviews, which carries this change with a test that covers the gate lines this one missed.
What changes
A pull's Reviewers menu said "Not a collaborator, so not counted" under review.knotless.dev's approval. The rule on main names it as a gate, so its review is exactly what the branch waits for. The menu now says "Required reviewer" for a gate, and keeps the old note for anyone else who may not write.
reviewViewgetsgate, true when the rule protecting the pull's base names the reviewer as one.No page the shots draw has a gate, so nothing should move.
Changeset: none
🤖 Generated with Claude Code