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.

Phase 3: a change request can be discussed

7c6e52e75f3a33be57a81663bc840edd6e14f7f9 · Keenan Nemetz · 17d ago · parent 3c5d11fa

Phase 3: a change request can be discussed

Until now the only ways to say something about a request were editing its body or
approving it. A review thread is now a pushed file like everything else:

	requests/<ulid>/comments/<ulid>.md    author + created, then the body

It is the *same file shape* as a comment on an issue and is read and written by the
same code (`issues.ParseComment`, `issues.RenderComment`, with `requests.Comment` as
an alias), so the thread inherited the validation, the markdown boundary and the
ownership rule instead of reimplementing them. That last one is the part worth
having: `pre-receive` compares the author on the old blob to the pushing account, so
a review comment is its author's to edit or delete, an admin may repair a thread
(audited), and rewriting the author line in the same push does not launder it.

`vole request comment <id> -m` prints the comment's id and the page anchor
(`#c-<ulid>`), `edit-comment` and `rm-comment` are the author's own words, and
`request show` prints the thread so the ids are discoverable at all — an id nobody
can see is an id nobody can use. Adding a comment is free, like adding one to an
issue: a new file's author is a claim, and the boundary has never pretended
otherwise.

The request's own file stays outside the ownership rule, for the reason an issue's
does: state, head and base are shared, and updating them is the author's normal edit.

Verified: TestParseReadsTheDiscussion (a thread plus an approval in one tree,
ordered by id), TestCommentNeedsAnAuthorAndATime and
TestUnknownFileInARequestIsRefused (the subtree stays a closed format, with both
expected shapes named), TestCommentPaths (a request comment is not an issue comment
and not an approval); and end to end through the binary,
TestRequestDiscussionIsPushedAndOwned — the empty state prints the command, the CLI
adds a comment, the page renders it with its anchor and `request show` lists it, a
writer is refused rewriting, adopting and deleting somebody else's comment, may join
the thread and may edit his own — with the issue-comment empty state assertion added
to TestIssuePushedThenRendered earlier. scripts/ci.sh green (gofmt, vet, build,
markdown links, go test -race ./...).

internal/requests/requests.go +31−2

@@ -40,13 +40,14 @@ const (
40 40
41 41 requestName = "request.md"
42 42 approvalDir = "approvals"
43 commentDir = "comments"
43 44 maxTitleLen = 200
44 45 )
45 46
46 47 var (
47 48 ulidRE = regexp.MustCompile(`^[0-9A-HJKMNP-TV-Z]{26}$`)
48 49 shaRE = regexp.MustCompile(`^[0-9a-f]{40}$`)
49 pathRE = regexp.MustCompile(`^requests/([0-9A-HJKMNP-TV-Z]{26})(?:/(request\.md)|/approvals/([0-9A-HJKMNP-TV-Z]{26})\.md)$`)
50 pathRE = regexp.MustCompile(`^requests/([0-9A-HJKMNP-TV-Z]{26})(?:/(request\.md)|/approvals/([0-9A-HJKMNP-TV-Z]{26})\.md|/comments/([0-9A-HJKMNP-TV-Z]{26})\.md)$`)
50 51 branchNameRE = regexp.MustCompile(`^[a-z0-9][a-z0-9._-]{0,62}$`)
51 52 )
52 53
@@ -61,8 +62,14 @@ type Request struct {
61 62 Created time.Time
62 63 Description string
63 64 Approvals []Approval
65 Comments []Comment
64 66 }
65 67
68 // Comment is a remark on a request. It is the same file shape as a comment on an
69 // issue — author, created, then the body — so one reader and one writer cover
70 // both, and the ownership rule at the push boundary applies to both.
71 type Comment = issues.Comment
72
66 73 type Approval struct {
67 74 Principal string
68 75 // Base is the branch the approver agreed the head should land on. It is part
@@ -142,7 +149,7 @@ func Parse(contents map[string]string) ([]*Request, error) {
142 149 m := pathRE.FindStringSubmatch(path)
143 150 if m == nil {
144 151 problems = append(problems, fmt.Sprintf(
145 "%s: not part of the request format (expected requests/<ulid>/request.md or approvals/<account>.md)", path))
152 "%s: not part of the request format (expected requests/<ulid>/request.md, approvals/<account>.md or comments/<ulid>.md)", path))
146 153 continue
147 154 }
148 155 id := m[1]
@@ -161,6 +168,15 @@ func Parse(contents map[string]string) ([]*Request, error) {
161 168 continue
162 169 }
163 170 req.Approvals = append(req.Approvals, a)
171 case m[4] != "":
172 // A comment on a request is read by the same code as a comment on an
173 // issue: one format, one reader, one set of refusals.
174 c, prob := issues.ParseComment(m[4], contents[path], path)
175 if prob != "" {
176 problems = append(problems, prob)
177 continue
178 }
179 req.Comments = append(req.Comments, c)
164 180 }
165 181 }
166 182
@@ -178,6 +194,7 @@ func Parse(contents map[string]string) ([]*Request, error) {
178 194 }
179 195 }
180 196 sort.Slice(req.Approvals, func(i, j int) bool { return req.Approvals[i].Principal < req.Approvals[j].Principal })
197 sort.Slice(req.Comments, func(i, j int) bool { return req.Comments[i].ID < req.Comments[j].ID })
181 198 out = append(out, req)
182 199 }
183 200 sort.Slice(out, func(i, j int) bool { return out[i].ID < out[j].ID })
@@ -278,6 +295,18 @@ func parseApproval(principal, raw, path string) (Approval, string) {
278 295 return a, ""
279 296 }
280 297
298 // CommentPath is where a comment on a request lives.
299 func CommentPath(requestID, commentID string) string {
300 return fmt.Sprintf("%s/%s/%s/%s.md", Dir, requestID, commentDir, commentID)
301 }
302
303 // IsCommentPath reports whether path is a comment on a request rather than its
304 // body or an approval. The hook uses it to apply comment authorship.
305 func IsCommentPath(path string) bool {
306 m := pathRE.FindStringSubmatch(path)
307 return m != nil && m[4] != ""
308 }
309
281 310 // ApprovalPath returns the principal an approvals/ path belongs to, or "".
282 311 func ApprovalPath(path string) string {
283 312 m := pathRE.FindStringSubmatch(path)