84 lines
3.9 KiB
Markdown
84 lines
3.9 KiB
Markdown
# PR#32: open hints hold the automerge of a GREEN pull request
|
|
|
|
> **WARNING:** This document describes only the change applied in this PR.
|
|
> It may already be outdated once the next PR is merged.
|
|
> Historic PR documentation is not maintained along with new PRs; treat it as a snapshot.
|
|
|
|
## The Problem
|
|
|
|
With `[gate] automerge = true`, werkrevisor 0.20.0 approves a GREEN pull request and schedules its merge at once.
|
|
A GREEN review can still hold hints: findings of low severity or doubted by Jev, posted as line comments.
|
|
Nobody gets a chance to answer them: PR#30 was merged 4 seconds after its GREEN review,
|
|
and its 3 hints went into PR#31 as additional changes.
|
|
|
|
## Non-Goals
|
|
|
|
- No change for YELLOW and ORANGE: a human approves them after reading the hints.
|
|
- No change of the verdict: a GREEN review with hints stays GREEN and approves.
|
|
- No new trigger for a review: the operator restarts the review build in Werkator, or the author pushes a commit.
|
|
|
|
## The Scenarios
|
|
|
|
### Feature: open hints hold the automerge
|
|
|
|
#### Scenario#32.01: a GREEN review with open hints approves, but schedules no merge
|
|
|
|
So that the author can answer the hints before the merge.
|
|
|
|
- **Given** `[gate] automerge = true`
|
|
- **When** werkrevisor reviews a head with the verdict GREEN and 2 hints as line comments
|
|
- **Then** the bot approves the head
|
|
- **and** it schedules no merge, and cancels a merge scheduled for an earlier head
|
|
- **and** the result comment says that the merge waits for 2 open hints
|
|
|
|
##### Verified by
|
|
|
|
- [test_hints_hold_merge: test_a_green_review_with_open_hints_approves_but_schedules_no_merge](../../tests/test_hints_hold_merge.py)
|
|
|
|
#### Scenario#32.02: a restarted review schedules the merge once all hints are resolved
|
|
|
|
So that the author can close the hints without a new commit.
|
|
|
|
- **Given** werkrevisor reviewed the head with GREEN and posted 2 hints as line comments
|
|
- **and** the author replied to both and resolved them
|
|
- **When** the review of the same head runs again
|
|
- **Then** the bot schedules the merge
|
|
|
|
##### Verified by
|
|
|
|
- [test_hints_hold_merge: test_a_restarted_review_schedules_the_merge_once_all_hints_are_resolved](../../tests/test_hints_hold_merge.py)
|
|
|
|
#### Scenario#32.03: the hold can be switched off
|
|
|
|
So that a repository can keep the merge at GREEN without waiting.
|
|
|
|
- **Given** `[gate] hints_hold_merge = false`
|
|
- **When** werkrevisor reviews a head with the verdict GREEN and 2 hints as line comments
|
|
- **Then** the bot approves and schedules the merge, as before
|
|
|
|
##### Verified by
|
|
|
|
- [test_hints_hold_merge: test_the_hold_can_be_switched_off](../../tests/test_hints_hold_merge.py)
|
|
|
|
## The Solution
|
|
|
|
- `[gate] hints_hold_merge`, default `true`, applies with `automerge`:
|
|
at GREEN, werkrevisor counts the unresolved line comments of the bot, from this and earlier rounds,
|
|
and the findings of this review that are no line comment, e.g. because Gitea refused them;
|
|
a restart reuses the verdict without findings, so it releases the latter.
|
|
- With open line comments, the bot approves, schedules no merge and cancels a merge scheduled earlier
|
|
(`DELETE /pulls/{n}/merge`, a 404 means none was scheduled);
|
|
the gate note, also a failed cancel, goes into the commit status and the result comment;
|
|
a failed cancel ends the review with exit code 10, so the failed gate check stops the merge scheduled earlier;
|
|
so do line comments that cannot be read, since unknown hints must not release the merge.
|
|
- A review of the same head, e.g. after a restart of its Werkator build, reuses the verdict, finds no open line comment
|
|
and schedules the merge; it posts a short comment with its gate note, since the result comment names the first review,
|
|
unless the merge was already scheduled.
|
|
- An unresolved line comment of an earlier round also holds the merge after a new commit;
|
|
the author resolves it once it is answered or obsolete.
|
|
- werkrevisor 0.20.1.
|
|
|
|
## Prerequisite PRs
|
|
|
|
- PR#31: excerpt selection by a cheap model.
|