Skip to content

Five Pre-Existing Defects in GOVERNANCE.md "PR Review Etiquette" #1239

Description

@ptr727

A whole-unit canonical pass over GOVERNANCE.md "PR Review Etiquette", run while resolving #1183, raised ten findings. Four were about the sentence #1183 changed and were fixed there. These five are about the unit's other paragraphs, predate that change, and are each a fleet-law edit that every carrying repository would take, so folding them into a PR about one pointer sentence would have put unrelated rule changes behind that PR's review.

1. The loop the unit mandates has no exit for a coverage state its own tool calls terminal

Confirm it covers the current head SHA and the full diff rather than only part of it. ... Repeat after every fix until the checks are green and the current-head review leaves no finding open. Only an explicit maintainer instruction may stop, defer, or alter this default.

scripts/pr_review.py documents exit 42, a round that read fewer files than the pull request changed, as a state to hand to the maintainer rather than one to retry into: "Measured over four pull requests and seven rounds here, a re-request never cleared one and no round ever recovered." The unit's only permitted stop is an explicit maintainer instruction, which a maintainer who has not been told about the state has no reason to have given. So a reader following the unit literally in that state either loops without end or breaks the rule.

2. The draft-state exception carves nothing out of the rule it qualifies

Open every fleet-owned pull request ready for review. Draft state is reserved for the separately documented upstream contribution workflow while a third-party contribution is still being prepared.

An upstream contribution is opened against a repository outside the fleet, so it is not a fleet-owned pull request and was never covered by the first sentence. The second sentence therefore excepts nothing, and the first stands as an unqualified absolute. Whether a fleet repository may ever open its own draft pull request is left unanswered rather than settled.

3. The upstream contribution workflow is named without being locatable

Draft state is reserved for the separately documented upstream contribution workflow

Named only as "separately documented", with no title, path, or file. The same unit locates the other package it names, "at .agents/skills/pr-review-conduct/SKILL.md in the hub". A reader holding only this file and a downstream checkout cannot reach the workflow this sentence defers to.

4. "Request a review on every push" is an absolute the implementation deliberately does not honor

scripts/pr_review.py's wait skips its auto-request where the reviewer is already requested, and .github/copilot-instructions.md states the reason: "Do not clear the request automatically because it may be active." A push that supersedes a still-pending request therefore gets no new request through the tool this unit routes to, so the instruction cannot be followed as written through the documented path.

5. "This is packaged as" opens a paragraph with no antecedent

This is packaged as the pr-review-conduct Skill at .agents/skills/pr-review-conduct/SKILL.md in the hub

The nearest noun is the preceding sentence's subject, the CLEAN merge state and the unanswered low-confidence findings it does not expose. The intended referent is the contract two sentences earlier. Naming the thing rather than pointing at it fixes it.

Note on how these were found

A subagent read the unit whole, as a repository carrying it for the first time reads it, which is the pass #1138 exists to move into this repository. The same read produced both halves and only one half had a home in the pull request that triggered it, which is the same split #1149 records.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    proseA defect in rule or procedure text

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions