diff --git a/.github/workflows/henokinho-review.yml b/.github/workflows/henokinho-review.yml index c2dda98..9c274d1 100644 --- a/.github/workflows/henokinho-review.yml +++ b/.github/workflows/henokinho-review.yml @@ -24,6 +24,26 @@ on: required: false type: string default: henokteixeira + label_name: + # Additive by construction rather than by this default. A caller reaches + # the label arm of the job's `if` only by putting `labeled` into its own + # `on: pull_request: types:`, so a caller that has not behaves exactly as + # it did before this input existed, whatever is written here. The default + # is the bot's own name rather than an empty string because a caller that + # does opt in and forgets to name a label would otherwise get a run that + # starts, skips, and looks identical to the guard working. + # + # This name is written twice on purpose. Each opted-in caller repeats it + # in a job-level `if` of its own, so that an unrelated label does not + # spend an invocation of this workflow merely to be turned away here — + # a cost that grows with every label a repository ever gains and that + # nobody notices, because the outcome looks the same either way. Nothing + # gates the two copies against each other, so each names the other: + # change this default and change the callers' guard with it. + description: 'A label of this name on a pull request triggers the bot, beside the review request and never instead of it' + required: false + type: string + default: henokinho app_id: description: 'GitHub App ID for the Henokinho App' required: true @@ -71,13 +91,35 @@ env: jobs: review: - # Fires only when the configured human reviewer is personally requested. - # A team request leaves requested_reviewer null, so this is safely false. + # Two ways to ask for this review, and both are decided here rather than in + # the callers, so a caller cannot loosen either by copying it wrong. + # + # The first is the configured human reviewer being personally requested. A + # team request leaves requested_reviewer null, so that arm is safely false + # for one — the bot answers a person being asked, never a team. + # + # The second is a label, and it exists for the case the first cannot cover: + # GitHub refuses a review request aimed at a pull request's own author — + # `422 Review cannot be requested from pull request author` — so on a + # repository where the stand-in's human opens the pull requests himself, + # requested_reviewer can never carry his login and the first arm can never + # fire. On a `labeled` event there is no requested_reviewer at all, which is + # why the login test is scoped to its own arm instead of governing both. + # + # Draft and bot-authored pull requests are excluded on either way in. + # + # Every line below sits at one indentation on purpose. `>-` folds only + # across lines indented alike; indent the inner clauses to read as nesting + # and YAML keeps their newlines, and what reaches GitHub is a multi-line + # expression rather than this one. if: >- - github.event.action == 'review_requested' && - github.event.requested_reviewer.login == inputs.reviewer_login && - github.event.pull_request.draft == false && - !endsWith(github.event.pull_request.user.login, '[bot]') + ( + (github.event.action == 'review_requested' && github.event.requested_reviewer.login == inputs.reviewer_login) + || + (github.event.action == 'labeled' && github.event.label.name == inputs.label_name) + ) + && github.event.pull_request.draft == false + && !endsWith(github.event.pull_request.user.login, '[bot]') runs-on: ubuntu-latest permissions: contents: read @@ -201,7 +243,8 @@ jobs: HEAD UNCHANGED SINCE THAT PASS: ${{ steps.prior.outputs.head_unchanged }} You are Henokinho, the automated first-pass reviewer for this org. You - run because @${{ inputs.reviewer_login }} was requested as a reviewer. + run because @${{ inputs.reviewer_login }} was requested as a reviewer, + or because the pull request was labelled `${{ inputs.label_name }}`. You go first so his own pass is cheaper. You do not replace it. READ THESE FILES IN FULL BEFORE REVIEWING ANYTHING. They are the spec, diff --git a/.github/workflows/joaozinho-review.yml b/.github/workflows/joaozinho-review.yml index 69a343d..dec37e8 100644 --- a/.github/workflows/joaozinho-review.yml +++ b/.github/workflows/joaozinho-review.yml @@ -19,6 +19,26 @@ on: required: false type: string default: joaocarvoli + label_name: + # Additive by construction rather than by this default. A caller reaches + # the label arm of the job's `if` only by putting `labeled` into its own + # `on: pull_request: types:`, so a caller that has not behaves exactly as + # it did before this input existed, whatever is written here. The default + # is the bot's own name rather than an empty string because a caller that + # does opt in and forgets to name a label would otherwise get a run that + # starts, skips, and looks identical to the guard working. + # + # This name is written twice on purpose. Each opted-in caller repeats it + # in a job-level `if` of its own, so that an unrelated label does not + # spend an invocation of this workflow merely to be turned away here — + # a cost that grows with every label a repository ever gains and that + # nobody notices, because the outcome looks the same either way. Nothing + # gates the two copies against each other, so each names the other: + # change this default and change the callers' guard with it. + description: 'A label of this name on a pull request triggers the bot, beside the review request and never instead of it' + required: false + type: string + default: joaozinho app_id: description: 'GitHub App ID for the Joãozinho App' required: true @@ -51,13 +71,35 @@ env: jobs: review: - # Fires only when the configured human reviewer is personally requested. - # A team request leaves requested_reviewer null, so this is safely false. + # Two ways to ask for this review, and both are decided here rather than in + # the callers, so a caller cannot loosen either by copying it wrong. + # + # The first is the configured human reviewer being personally requested. A + # team request leaves requested_reviewer null, so that arm is safely false + # for one — the bot answers a person being asked, never a team. + # + # The second is a label, and it exists for the case the first cannot cover: + # GitHub refuses a review request aimed at a pull request's own author — + # `422 Review cannot be requested from pull request author` — so on a + # repository where the stand-in's human opens the pull requests himself, + # requested_reviewer can never carry his login and the first arm can never + # fire. On a `labeled` event there is no requested_reviewer at all, which is + # why the login test is scoped to its own arm instead of governing both. + # + # Draft and bot-authored pull requests are excluded on either way in. + # + # Every line below sits at one indentation on purpose. `>-` folds only + # across lines indented alike; indent the inner clauses to read as nesting + # and YAML keeps their newlines, and what reaches GitHub is a multi-line + # expression rather than this one. if: >- - github.event.action == 'review_requested' && - github.event.requested_reviewer.login == inputs.reviewer_login && - github.event.pull_request.draft == false && - !endsWith(github.event.pull_request.user.login, '[bot]') + ( + (github.event.action == 'review_requested' && github.event.requested_reviewer.login == inputs.reviewer_login) + || + (github.event.action == 'labeled' && github.event.label.name == inputs.label_name) + ) + && github.event.pull_request.draft == false + && !endsWith(github.event.pull_request.user.login, '[bot]') runs-on: ubuntu-latest permissions: contents: read @@ -196,7 +238,8 @@ jobs: HEAD UNCHANGED SINCE YOUR LAST PASS: ${{ steps.prior.outputs.head_unchanged }} You are Joãozinho, the automated first-pass reviewer for this org. You - run because @${{ inputs.reviewer_login }} was requested as a reviewer. + run because @${{ inputs.reviewer_login }} was requested as a reviewer, + or because the pull request was labelled `${{ inputs.label_name }}`. You go first so his own pass is cheaper. You do not replace it. READ THESE THREE FILES IN FULL BEFORE REVIEWING ANYTHING. They are the diff --git a/README.md b/README.md index 15448d0..0fe11b2 100644 --- a/README.md +++ b/README.md @@ -7,9 +7,18 @@ reusable workflows that every product repo calls. ## Automated PR reviewers Two bots review pull requests across the org. Each one stands in for a specific -human and runs **only when that person is personally requested as a reviewer** — -a request aimed at a team does not fire either of them, and neither runs on a -draft PR or on a PR opened by a bot. +human, and there are **two ways to ask for one**: request that person as a +reviewer, or label the pull request with the bot's own name. A request aimed at +a team fires neither of them, and neither runs on a draft PR or on a PR opened +by a bot. + +The label is there for the case a review request cannot reach. GitHub refuses a +request aimed at a pull request's own author — `422 Review cannot be requested +from pull request author` — so on a repository where the stand-in's human opens +the pull requests himself, there is no login left to request and the bot can +never fire. The name is the `label_name` input and defaults to the bot's own. +**A caller only gets that second way in by putting `labeled` into its own +`on: pull_request: types:`**; a caller that has not is unchanged. | Bot | Stands in for | Reusable workflow | Spec | |---|---|---|---| diff --git a/henokinho/review-style.md b/henokinho/review-style.md index d3b8de7..123b7d0 100644 --- a/henokinho/review-style.md +++ b/henokinho/review-style.md @@ -5,9 +5,11 @@ You are **Henokinho**, a first-pass reviewer standing in for Henok background reading. The stack-specific lens pack loaded alongside it tells you what the lenses mean in *this* repo. -You run when he is personally requested as a reviewer. You go first so his own -pass is cheaper. **You do not replace it, you never approve, and you never -block.** Inline comments plus one COMMENT review — approval is his and only his. +You run when he is personally requested as a reviewer, or when a pull request is +labelled for you — the way in on a repository where he opens the pull requests +himself and GitHub will not let him be asked. You go first so his own pass is +cheaper. **You do not replace it, you never approve, and you never block.** +Inline comments plus one COMMENT review — approval is his and only his. Write in **English**, always. Portuguese domain vocabulary stays in Portuguese where the product uses it (Ritmo, Pulso, Oração, Equipe, telha, verde).