Sign in

Search issue titles and bodies, change requests and comments. Private repositories you cannot read are not included.
vole
Clone

Read only, no credentials:

https://rodent.software/git/vole.git

Write over SSH with a registered key:

git clone ssh://git@0.0.0.0:2222/vole.git

A free and open-source forge that lives in your Git repo.

Blame · docs/change-requests.md

Line attribution on main · 344 line(s)

View file History
520470e1
Keenan Nemetz
16d ago
 1 # Change requests
2
3 Review should not depend on a web session or a vendor. In Vole, a change request
e050e14a
Keenan Nemetz
13d ago
 4 is a normal branch and a normal push. A request opened from a branch follows that
5 branch: every push moves the request's head, so an approval always names the exact
6 commit it reviewed and a moved branch starts the review over.
520470e1
Keenan Nemetz
16d ago
 7
8 ## Files behind a request
9
10 ```text
b492b557
Keenan Nemetz
3d ago
 11 changes/<ulid>/change.md # title, state, author, base, head, closes
12 changes/<ulid>/comments/<ulid>.md # discussion
13 changes/<ulid>/approvals/<account-name>.md # one approval per account
520470e1
Keenan Nemetz
16d ago
 14 ```
15
16 ## Opening and landing a request
17
a781cd9b
Keenan Nemetz
6d ago
 18 A request starts from a branch you may push. Which branches those are depends on
19 your role:
20
21 - A `maintain` or `admin` account may push shared branches, so it can propose any
22 ordinary branch in the repository.
23 - A `contribute` account may propose only a branch in its own namespace,
24 `refs/heads/contrib/<name>/`.
25
26 The **Open change request** form on the repository's change-request page lists
27 exactly the branches your account may propose; pre-receive checks the branch the
28 request names the same way. A `contribute` account cannot name an arbitrary
29 commit or another account's branch. The namespace is the account name, which is
30 fixed at creation. CI for a branch under `refs/heads/contrib/` is held until a
31 maintainer allows the exact tip from the repository page; a shared branch runs CI
32 when it is pushed.
33
802e0e52
Keenan Nemetz
4d ago
 34 The change's author or a maintainer can correct its title with the pencil beside
35 the heading, or its description from the card's menu. Each opens its own field in
36 place, so saving does not leave the page or replace the review. The prose is not
37 what an approval names — the base and head are — so the review and its approvals
38 stay as they are, and a closed or merged record can be corrected too.
a4e7a99a
Keenan Nemetz
5d ago
 39
a781cd9b
Keenan Nemetz
6d ago
 40 A maintainer can open a request from a shared branch with the CLI:
6c84d387
Keenan Nemetz
15d ago
 41
520470e1
Keenan Nemetz
16d ago
 42 ```sh
a0a7c73a
Keenan Nemetz
13d ago
 43 # The author pushes a branch and opens a request for the issue it fixes.
520470e1
Keenan Nemetz
16d ago
 44 git push origin feature:refs/heads/feature
a0a7c73a
Keenan Nemetz
13d ago
 45 vole change new --title "Retry on timeout" --base main --head feature \
46 --closes 01M5JR2JK29KKMAA1T9MDNTPBP -m "Rationale."
520470e1
Keenan Nemetz
16d ago
 47
48 # An approver reviews that commit and approves it.
04e00cfc
Keenan Nemetz
15d ago
 49 vole change approve <id> --as <your-name>
520470e1
Keenan Nemetz
16d ago
 50
51 # The author lands it. Vole checks that the approval still matches the new tip.
52 git push origin feature:refs/heads/main
53 ```
54
e050e14a
Keenan Nemetz
13d ago
 55 The request records the branch it follows in `head_ref:` and the commit currently
6755b751
Keenan Nemetz
9d ago
 56 under review in `head:`. A branch is resolved on the remote, not in the local
57 clone: if the clone is behind, the request records the remote tip and says so on
58 stderr, because that is the commit the forge will review and land. A branch that
59 is only local is recorded too, with a note that it is not on the remote yet. A
60 request whose head is already in its base — the same commit, or an ancestor — is
61 refused: it would review nothing. The request page, the request list and
62 `vole change show` show both. After every accepted push, Vole moves `head:` to the new tip of the
e050e14a
Keenan Nemetz
13d ago
 63 followed branch, so a fixup does not need a second command. Approvals and checks
64 still name a commit: the approvals that named the old tip stop counting, and the new
65 tip needs its own CI result. An approval is never carried silently to a commit nobody
66 reviewed — the approver's form names the base and head it displayed, and the writer
67 refuses if a push or a retarget moved either one in between.
68
69 A request opened from a bare commit id records no branch and stays pinned to that
70 commit. `vole change update <id> --head <branch>` makes an open request follow a
71 branch (or a different one); `--head <commit>` pins it to one commit. The request
41d1fee9
Keenan Nemetz
13d ago
 72 page's base/head picker does the same. Neither is needed after a normal push: the
73 request already follows the branch it was opened from. A branch that is deleted
74 leaves the last tip it saw in `head:` and the page says the branch is gone.
8e1005eb
Keenan Nemetz
13d ago
 75
520470e1
Keenan Nemetz
16d ago
 76 Other commands:
77
78 ```sh
04e00cfc
Keenan Nemetz
15d ago
 79 vole change list [--state open|closed]
80 vole change show <id>
e050e14a
Keenan Nemetz
13d ago
 81 vole change update <id> [--base <branch>] [--head <rev>] [--closes <issue>]... [--no-closes]
04e00cfc
Keenan Nemetz
15d ago
 82 vole change close <id>
83 vole change reopen <id>
520470e1
Keenan Nemetz
16d ago
 84 ```
85
04e00cfc
Keenan Nemetz
15d ago
 86 `vole change show <id>` also tells you whether the branch still merges, when the
520470e1
Keenan Nemetz
16d ago
 87 needed commits are available locally. If they are not, it says so rather than
88 guessing.
89
e050e14a
Keenan Nemetz
13d ago
 90 ## Stacked requests
91
92 A request may target another feature branch instead of `main`. Push the lower
93 branch first, then open the upper request with `--base <lower-branch> --head
94 <upper-branch>`. Its diff is only the upper change, not everything the lower branch
95 adds as well.
96
97 When the lower request lands, retarget the upper one:
98
99 ```sh
100 vole change update <id> --base main
101 ```
102
41d1fee9
Keenan Nemetz
13d ago
 103 The request page's target picker changes the base the same way. A new base changes
104 the diff, and every approval named the old base, so the upper request needs a fresh
e050e14a
Keenan Nemetz
13d ago
 105 review against the new base. Changing the base does not change the branch the
41d1fee9
Keenan Nemetz
13d ago
 106 request follows; pick a different head branch too only if the upper branch itself
107 moved.
e050e14a
Keenan Nemetz
13d ago
 108
e535ca55
Keenan Nemetz
4d ago
 109 A landing moves the base branch without a push, and Vole still moves the open
110 requests that follow that branch onto the landed commit — the same rule a push to
111 the branch applies. So a lower request follows its branch when an upper request
112 lands into it, with no separate command.
113
a0a7c73a
Keenan Nemetz
13d ago
 114 ## Linking the issues it closes
115
116 A change request names the issues it fixes in its own metadata, not in its
b492b557
Keenan Nemetz
3d ago
 117 description: `change.md` carries a `closes` list, the request page shows those
a0a7c73a
Keenan Nemetz
13d ago
 118 issues, and landing the request closes them in the same commit that marks it
e05f1c48
Keenan Nemetz
13d ago
 119 merged. A `#42`, `#<ulid>` or `Fixes #42` in the description is only prose — it
120 does not link anything and closes nothing.
a0a7c73a
Keenan Nemetz
13d ago
 121
122 Set the list when the request is opened, or later while it is still open:
123
124 ```sh
125 vole change new --title "Retry on timeout" --base main --head feature \
126 --closes 01M5JR2JK29KKMAA1T9MDNTPBP,01M5N1PRC4GJPZNEEG8PR8J4D6
e05f1c48
Keenan Nemetz
13d ago
 127 vole change new --title "Retry on timeout" --base main --head feature \
128 --closes 42,43 # the same list by number
129 vole change update <id> --closes 42 # replace the list
130 vole change update <id> --no-closes # clear it
a0a7c73a
Keenan Nemetz
13d ago
 131 ```
132
e05f1c48
Keenan Nemetz
13d ago
 133 The flag takes a full ULID or the `#42` number, and `42` is resolved to the ULID
134 before the link is written, so the file always holds ids. The issue has to exist
135 when the link is written, so a typo fails while the author is still looking at the
136 command, and a number that names a change request is refused: a request cannot
137 close a request. The browser's **Open change request** form takes the same list in
138 its **Closes** field.
139
140 A number is shared with issues, so `#42` may name either kind and the server
141 resolves it when the link is rendered. `closes:` is not where that resolution
142 lives: it holds ULIDs, because that is the reference that survives a rewritten
143 history.
a0a7c73a
Keenan Nemetz
13d ago
 144
0789f22e
Keenan Nemetz
9d ago
 145 A repository can require a linked issue for every new change request. The
146 checkbox is in **Settings → Access**, and the same rule is available to an
147 instance admin from the terminal:
148
149 ```sh
150 vole admin acl require-issues enable demo
151 ```
152
153 The request page marks **Closes** as required, the browser refuses an unlinked
154 request, and the push gate refuses one added through the CLI or a hand-written
155 metadata commit. Existing requests are unaffected: turning the setting on does
156 not invalidate history, and turning it off allows new requests with no link
157 again.
158
a0a7c73a
Keenan Nemetz
13d ago
 159 Only an open request may be relinked: a merged request's list records what the
267feec2
Keenan Nemetz
10d ago
 160 merge did, and a closed one is not going to land. The request page's **Issues**
838f282e
Keenan Nemetz
10d ago
 161 section lists the linked issues in the same rows as the overview's recent issues.
162 Its search box finds issues by title, `#number`, or ULID; picking a result links
163 it, and the unlink icon removes one. Both controls use the same `closes` list as
164 the CLI, and only the request's author or a maintainer may change it. Landing is
165 what closes an issue; linking one does not, and a request that is closed without
166 landing leaves its issues open.
a0a7c73a
Keenan Nemetz
13d ago
 167
520470e1
Keenan Nemetz
16d ago
 168 ## How approvals work
169
e5478085
Keenan Nemetz
11d ago
 170 A protected branch only accepts an update that carries the reviewed head. When
171 nothing else has landed, the update is the head itself. When the branch has moved,
172 Vole builds the merge, tests that, and lands the commit it tested.
173
174 The approval names the base branch and the head, not the commit the base currently
175 points at. A base move therefore retests the combined result but does not invalidate
176 the approval: nobody has to rebase or re-approve just because something else landed
177 first.
178
179 Adding a commit to the branch after approval still does not carry the old approval.
180 The request follows its branch, so the push moves its head and the approval names a
181 commit that is no longer the head.
182
183 "Merge, then push" is accepted only when the pushed merge commit has the tree a
184 clean merge of the branch tip and the reviewed head would produce, and the combined
185 result has passed. A merge commit with extra content, or with conflicts resolved by
186 hand, is refused: it is content nobody reviewed under that approval. The **Merge**
187 button and `vole change merge` are the supported path.
188
189 ## The combined result
190
191 For a landing that needs a merge commit, Vole builds it and runs the repository's
192 workflows against that tree. A combined result is tested when a head is pushed, and
193 again when the base moves if the request is under review or already waiting to
194 merge. The server looks at open requests on a short interval, so a burst of
195 work-in-progress pushes settles on the head that is actually there before a run is
196 queued.
197
198 The combined result uses the workflow files at the merge, so a workflow change on
199 the base applies to the landing. It is a normal run for the base branch, but it does
5f6163bb
Keenan Nemetz
1h ago
 200 not deploy the repository's site.
201
202 A landing lands that same commit, so the push that moves the branch does not run
203 those jobs a second time: the tree, the workflow and the job already ran for this
204 branch, and the run list shows the run that tested it once. A job that deploys the
205 site is the exception — a combined result never deploys — so that one still runs on
206 the branch, because publishing the site is work the combined result did not do. A combined result derived from a contributor
e5478085
Keenan Nemetz
11d ago
 207 branch is still untrusted code: it runs only after that exact head was allowed, and
208 it uses the contributor's cache namespace.
209
210 The request page shows one status:
211
212 - **Passed with the latest main** — Merge lands immediately.
213 - **Testing the merge with the latest main** — Merge becomes **Merge when ready**
214 and lands as soon as the run passes.
215 - **The combined result failed** — the runs name the job. Pushing a fix starts a new
216 review, because the head changes.
217 - **Conflicts with the latest main** — resolve them on the branch; the server does
218 not invent a resolution.
219
220 **Merge when ready** survives the base moving. It stops when the head changes, the
221 request closes, the actor loses merge permission, or the request conflicts. Every
222 landing re-checks the approvals, the actor's permission, and the combined result at
223 the moment the branch moves. `vole change merge` does the same thing from the
224 terminal, and `vole change show` prints the combined result and whether it is ready.
520470e1
Keenan Nemetz
16d ago
 225
6c84d387
Keenan Nemetz
15d ago
 226 An approval can also be recorded from the request page in the browser when the
62576fa8
Keenan Nemetz
14d ago
 227 signed-in account has maintain access. The approver is the session's account name, not
e050e14a
Keenan Nemetz
13d ago
 228 a name from the page. The page's Approve form carries the base and head it
229 displayed; if a push moved the branch, or the request was retargeted, before the form
230 was submitted, the approval is refused and the approver sees the new target instead
231 of approving it unseen.
6c84d387
Keenan Nemetz
15d ago
 232
62576fa8
Keenan Nemetz
14d ago
 233 An approval is stored at `approvals/<account-name>.md`. Only the account named in
b492b557
Keenan Nemetz
3d ago
 234 that path can create, change, or remove it. `change.md` records the author's
62576fa8
Keenan Nemetz
14d ago
 235 account name in `author`; that is the only author field and it is what blocks
236 self-approval. An admin can repair an
474a812d
Keenan Nemetz
15d ago
 237 approval, and the override is recorded in `data/audit.log`.
520470e1
Keenan Nemetz
16d ago
 238
62b59a76
Keenan Nemetz
12d ago
 239 An approval counts only while its author still has the standing to give it: the
240 account must be active and must currently hold `maintain` or `admin` on the
241 repository. Retiring an account or reducing its role therefore withdraws its
242 approvals without editing any file — the file stays on the branch, but the gate
243 stops counting it, and the page shows it as **no longer counts** rather than as a
244 review the landing will accept. The page and the gate use the same rule, so a
245 request page cannot promise a merge that the push then refuses.
246
f6f4b1db
Keenan Nemetz
15d ago
 247 The request page has three tabs. **Conversation** holds the description,
248 discussion, approvals and checks. **Files** is the review surface: a tree of the
249 changed files beside the selected file's diff, with line comments and their
250 composer in place. **Runs** lists the workflow results recorded for the head
c765ae2e
Keenan Nemetz
13d ago
 251 commit, with each run's duration and attempts, and links to the run page, step
252 logs and full log.
f6f4b1db
Keenan Nemetz
15d ago
 253
254 Together they show:
520470e1
Keenan Nemetz
16d ago
 255
256 - the changes compared with the branch it would merge into,
257 - CI checks for the head commit,
e5478085
Keenan Nemetz
11d ago
 258 - the combined result that would land, and whether it has passed,
520470e1
Keenan Nemetz
16d ago
 259 - how many approvals are required and who approved which commit,
260 - whether the branch still merges cleanly.
261
262 Vole checks the last item with Git itself, without touching a working copy. It is
263 shown for information, not enforced. Vole leaves it out when it cannot answer — for
264 example, if the base branch no longer exists.
265
2841d922
Keenan Nemetz
13d ago
 266 A merged request keeps showing the change it landed. The mark records
267 `merged_base`, the tip the base pointed at before it moved, because afterwards the
268 head is in the base and a diff of the two is empty. That is what the Files tab
269 diffs when the request is already merged.
270
7cc36ba0
Keenan Nemetz
4d ago
 271 ## The change list
272
273 The change list shows one status per row, so the queue can be read without
274 opening every change:
275
276 - **Waiting on CI approval** — a contributor branch at this commit has not been
277 allowed by a maintainer yet.
278 - **Waiting on CI jobs** — checks for the commit are queued or running.
279 - **CI failed** — a check failed; pushing a fix starts a new review.
70c9ad4e
Keenan Nemetz
1h ago
 280 - **CI could not run** — a runner could not start a job for this commit, so nothing
281 was tested. The commit is not what failed: rerun the job once the machine can
282 build, and the check takes the new result.
7cc36ba0
Keenan Nemetz
4d ago
 283 - **Waiting for review** — fewer current approvals than the base branch requires.
284 - **Merge conflict** — the branch no longer merges into its base cleanly.
285 - **Ready to merge** — the required approvals are in (or none are required) and
286 the commit's checks passed.
287 - **Merged**, **Closed** — the change landed, or was closed without landing.
288
289 The list also shows when each change was last updated: the newest commit that
290 touched its directory, which covers an edit, a comment, and an approval. It sorts
291 by that, so the change that moved most recently is first.
292
520470e1
Keenan Nemetz
16d ago
 293 ## Review comments
294
295 ```sh
04e00cfc
Keenan Nemetz
15d ago
 296 vole change comment <id> -m "Does this hold for the empty repository case?"
297 vole change comment <id> -m "Good catch, amended." # prints the comment id
f6f4b1db
Keenan Nemetz
15d ago
 298 vole change comment <id> --reply-to <comment-id> -m "A follow-up."
04e00cfc
Keenan Nemetz
15d ago
 299 vole change edit-comment <id> <comment-id> -m "Rephrased."
300 vole change rm-comment <id> <comment-id>
520470e1
Keenan Nemetz
16d ago
 301 ```
302
f6f4b1db
Keenan Nemetz
15d ago
 303 A comment can reply to any comment with `--reply-to`. The web shows one level of
304 nesting by collecting a root's descendants under it. Each comment has a context menu with
305 Reply, Copy, Edit and Delete. Reply targets the one composer at the foot of the
306 discussion; **Cancel reply** clears the target without clearing the draft. Replies
307 can be folded under their root, while the root comment stays visible. Editing and
308 deleting are limited to the author or an admin. Deleting a comment leaves a
309 tombstone so its replies keep the thread they answer.
310
520470e1
Keenan Nemetz
16d ago
 311 Only the author can edit or delete a comment. An admin can override that, and the
f6f4b1db
Keenan Nemetz
15d ago
 312 override is recorded in `data/audit.log`. Comments appear in the Conversation tab
313 with an anchor for each one (`#c-<ulid>`); a line comment also appears beneath its
314 line in the Files tab (`#inline-c-<ulid>`). A line comment in the conversation
315 carries a collapsed excerpt of the lines around it, read from the exact blob the
316 comment named, so a stale comment still shows the code that was reviewed.
520470e1
Keenan Nemetz
16d ago
 317
318 You can attach a comment to a line on either side of the diff:
319
320 ```sh
04e00cfc
Keenan Nemetz
15d ago
 321 vole change comment <id> -m "this drops the check" \
520470e1
Keenan Nemetz
16d ago
 322 --path internal/auth.go --line 88
04e00cfc
Keenan Nemetz
15d ago
 323 vole change comment <id> -m "and this was the old line" \
520470e1
Keenan Nemetz
16d ago
 324 --path internal/auth.go --line 88 --side old
325 ```
326
327 The comment remembers the version of the file it was written about, not just the
328 line number. If the file changes, the web page marks the comment as out of date.
329 The CLI needs the commits in your clone; if they are missing, it tells you to run
330 `git fetch` instead of guessing.
331
332 ## Branch rules
333
334 ```sh
2e024cc9
Keenan Nemetz
14d ago
 335 vole admin acl protect add demo refs/heads/main # protect a branch
336 vole admin acl set-approvals demo 2 # require two approvals
337 vole admin acl set-approvals demo 0 # back to admin-only
338 vole admin acl checks enable demo # also require passing CI checks
520470e1
Keenan Nemetz
16d ago
 339 ```
340
341 Admins can push a protected branch directly. Every merge through review is
342 recorded in `data/audit.log` with the request id and approvers. For the runner
70b70264
Keenan Nemetz
15d ago
 343 allowlist behind required checks, see
344 [Runners](runner.md#who-can-run-jobs-and-report-results).