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.

Issues and requests are frontmatter, not JSON

307cbc2aa624ca9b76b79849e8ce8926414be435 · Keenan Nemetz · 17d ago · parent 427c4761

Issues and requests are frontmatter, not JSON

issues/<ulid>/issue.md and requests/<ulid>/request.md replace meta.json plus
description.md, and approvals/<account>.md replaces approvals/<account>.json.
Every file on refs/heads/vole is now frontmatter markdown read by one package, so
there is one reader and one writer for issues, requests, comments and approvals
instead of a JSON path and a hand-rolled key/value path.

The diff is the interface. A JSON array rewrites its neighbour to add a comma, so
one label cost two changed lines and removing one cost three; a block sequence
costs one. docs/spikes/issue-frontmatter measured the same six edits at 25 changed
lines as meta.json and 10 as frontmatter, and one file per entity means one path
to diff, blame and log.

Three rules make YAML safe to hand-edit rather than something to police:

- the writer always double-quotes free text, so no indicator, reserved word or
  " # " can change what a title means
- a label is quoted only when the parser would not read it back bare ("null"),
  which leaves ordinary labels as they were
- a reader refuses an unquoted title that the parser read differently: "title:
  fix #12 crash" would otherwise be stored as "fix" and nobody would find out

Control characters are refused on both sides, so a tree the server accepts is a
tree it can always rewrite.

`updated` is gone from issues, requests and the index's issues table. It cost a
changed line on every edit — 11 of the 21 frontmatter lines in the spike's
sequence — and the commit that changed the file already carries the time.

Verified: TestRenderParseRoundTrip puts 13 titles that break naive YAML (hash,
colon, leading dash, null, quotes, backslash, unicode, 190 bytes) through the
writer and the reader; the server tests push a rendered issue over SSH and render
it, and the approval-ownership tests withdraw approvals/<account>.md. Then
scripts/ci.sh green (gofmt, vet, build, markdown links, go test -race ./...).

internal/requests/requests.go +53−46

@@ -2,9 +2,12 @@
2 2 // plus the approvals that allow it through.
3 3 //
4 4 // refs/heads/vole
5 // requests/<ulid>/meta.json
6 // requests/<ulid>/description.md
7 // requests/<ulid>/approvals/<principal>.json
5 // requests/<ulid>/request.md frontmatter + body
6 // requests/<ulid>/approvals/<principal>.md frontmatter, no body
7 //
8 // Every file on the metadata branch is frontmatter markdown, so one reader and
9 // one writer cover issues, change requests, comments and approvals. A record
10 // with no prose (an approval) ends at the closing fence.
8 11 //
9 12 // An approval names the exact commit it reviewed. That is the whole point: a
10 13 // general "I approve this request" is worthless, because the head can move
@@ -17,13 +20,13 @@
17 20 package requests
18 21
19 22 import (
20 "encoding/json"
21 23 "fmt"
22 24 "regexp"
23 25 "sort"
24 26 "strings"
25 27 "time"
26 28
29 "github.com/rodent-software/vole/internal/frontmatter"
27 30 "github.com/rodent-software/vole/internal/gitx"
28 31 "github.com/rodent-software/vole/internal/id"
29 32 "github.com/rodent-software/vole/internal/issues"
@@ -35,8 +38,7 @@ const (
35 38 // Ref is the metadata branch, shared with issues.
36 39 Ref = issues.Ref
37 40
38 metaName = "meta.json"
39 descName = "description.md"
41 requestName = "request.md"
40 42 approvalDir = "approvals"
41 43 maxTitleLen = 200
42 44 )
@@ -44,7 +46,7 @@ const (
44 46 var (
45 47 ulidRE = regexp.MustCompile(`^[0-9A-HJKMNP-TV-Z]{26}$`)
46 48 shaRE = regexp.MustCompile(`^[0-9a-f]{40}$`)
47 pathRE = regexp.MustCompile(`^requests/([0-9A-HJKMNP-TV-Z]{26})(?:/(meta\.json|description\.md)|/approvals/([0-9A-HJKMNP-TV-Z]{26})\.json)$`)
49 pathRE = regexp.MustCompile(`^requests/([0-9A-HJKMNP-TV-Z]{26})(?:/(request\.md)|/approvals/([0-9A-HJKMNP-TV-Z]{26})\.md)$`)
48 50 branchNameRE = regexp.MustCompile(`^[a-z0-9][a-z0-9._-]{0,62}$`)
49 51 )
50 52
@@ -57,7 +59,6 @@ type Request struct {
57 59 Head string // commit sha under review
58 60 State string // open | merged | closed
59 61 Created time.Time
60 Updated time.Time
61 62 Description string
62 63 Approvals []Approval
63 64 }
@@ -84,15 +85,16 @@ func (r *Request) ApprovedBy(principal string) bool {
84 85 return false
85 86 }
86 87
88 // meta is the frontmatter head of request.md. Field names are part of the
89 // public format. There is no updated field: the commit carries the time.
87 90 type meta struct {
88 Title string `json:"title"`
89 Author string `json:"author"`
90 AuthorID string `json:"author_id"`
91 Base string `json:"base"`
92 Head string `json:"head"`
93 State string `json:"state"`
94 Created string `json:"created"`
95 Updated string `json:"updated,omitempty"`
91 Title string `yaml:"title"`
92 Author string `yaml:"author"`
93 AuthorID string `yaml:"author_id"`
94 Base string `yaml:"base"`
95 Head string `yaml:"head"`
96 State string `yaml:"state"`
97 Created string `yaml:"created"`
96 98 }
97 99
98 100 // Load reads and validates every request on the metadata branch.
@@ -140,7 +142,7 @@ func Parse(contents map[string]string) ([]*Request, error) {
140 142 m := pathRE.FindStringSubmatch(path)
141 143 if m == nil {
142 144 problems = append(problems, fmt.Sprintf(
143 "%s: not part of the request format (expected requests/<ulid>/meta.json, description.md or approvals/<user>.json)", path))
145 "%s: not part of the request format (expected requests/<ulid>/request.md or approvals/<account>.md)", path))
144 146 continue
145 147 }
146 148 id := m[1]
@@ -150,10 +152,8 @@ func Parse(contents map[string]string) ([]*Request, error) {
150 152 byID[id] = req
151 153 }
152 154 switch {
153 case m[2] == metaName:
154 applyMeta(req, contents[path], path, &problems)
155 case m[2] == descName:
156 req.Description = contents[path]
155 case m[2] == requestName:
156 applyRequest(req, contents[path], path, &problems)
157 157 case m[3] != "":
158 158 a, prob := parseApproval(m[3], contents[path], path)
159 159 if prob != "" {
@@ -167,14 +167,14 @@ func Parse(contents map[string]string) ([]*Request, error) {
167 167 out := make([]*Request, 0, len(byID))
168 168 for _, req := range byID {
169 169 if req.Title == "" {
170 problems = append(problems, fmt.Sprintf("%s/%s/%s: title is required", Dir, req.ID, metaName))
170 problems = append(problems, fmt.Sprintf("%s/%s/%s: title is required", Dir, req.ID, requestName))
171 171 }
172 172 // Self-approval is not an approval. Catching it here rather than at the
173 173 // gate means the author learns immediately instead of after a rejection.
174 174 for _, a := range req.Approvals {
175 175 if a.Principal == req.AuthorID {
176 176 problems = append(problems, fmt.Sprintf(
177 "%s/%s/%s/%s.json: the author cannot approve their own request", Dir, req.ID, approvalDir, a.Principal))
177 "%s/%s/%s/%s.md: the author cannot approve their own request", Dir, req.ID, approvalDir, a.Principal))
178 178 }
179 179 }
180 180 sort.Slice(req.Approvals, func(i, j int) bool { return req.Approvals[i].Principal < req.Approvals[j].Principal })
@@ -188,14 +188,23 @@ func Parse(contents map[string]string) ([]*Request, error) {
188 188 return out, nil
189 189 }
190 190
191 func applyMeta(req *Request, raw, path string, problems *[]string) {
191 func applyRequest(req *Request, raw, path string, problems *[]string) {
192 head, body, err := frontmatter.Split([]byte(raw))
193 if err != nil {
194 *problems = append(*problems, fmt.Sprintf("%s: %v", path, err))
195 return
196 }
192 197 var m meta
193 dec := json.NewDecoder(strings.NewReader(raw))
194 dec.DisallowUnknownFields()
195 if err := dec.Decode(&m); err != nil {
196 *problems = append(*problems, fmt.Sprintf("%s: cannot parse: %v", path, err))
198 if err := frontmatter.Decode(head, &m); err != nil {
199 *problems = append(*problems, fmt.Sprintf("%s: cannot parse frontmatter: %v", path, err))
197 200 return
198 201 }
202 if err := frontmatter.Text(head, "title", m.Title); err != nil {
203 *problems = append(*problems, fmt.Sprintf("%s: title: %v", path, err))
204 }
205 if err := frontmatter.Text(head, "author", m.Author); err != nil {
206 *problems = append(*problems, fmt.Sprintf("%s: author: %v", path, err))
207 }
199 208 req.Title = strings.TrimSpace(m.Title)
200 209 if len(req.Title) > maxTitleLen {
201 210 *problems = append(*problems, fmt.Sprintf("%s: title is %d bytes, limit %d", path, len(req.Title), maxTitleLen))
@@ -233,11 +242,7 @@ func applyMeta(req *Request, raw, path string, problems *[]string) {
233 242 } else {
234 243 req.Created = t.UTC()
235 244 }
236 if m.Updated != "" {
237 if t, err := time.Parse(time.RFC3339, m.Updated); err == nil {
238 req.Updated = t.UTC()
239 }
240 }
245 req.Description = body
241 246 }
242 247
243 248 func parseApproval(principal, raw, path string) (Approval, string) {
@@ -245,25 +250,27 @@ func parseApproval(principal, raw, path string) (Approval, string) {
245 250 if !id.Valid(principal) {
246 251 return a, fmt.Sprintf("%s: the file name must be the approver's account id (a 26-character ULID)", path)
247 252 }
248 var body struct {
249 Base string `json:"base"`
250 Head string `json:"head"`
251 Created string `json:"created"`
253 head, _, err := frontmatter.Split([]byte(raw))
254 if err != nil {
255 return a, fmt.Sprintf("%s: %v", path, err)
256 }
257 var m struct {
258 Base string `yaml:"base"`
259 Head string `yaml:"head"`
260 Created string `yaml:"created"`
252 261 }
253 dec := json.NewDecoder(strings.NewReader(raw))
254 dec.DisallowUnknownFields()
255 if err := dec.Decode(&body); err != nil {
256 return a, fmt.Sprintf("%s: cannot parse: %v", path, err)
262 if err := frontmatter.Decode(head, &m); err != nil {
263 return a, fmt.Sprintf("%s: cannot parse frontmatter: %v", path, err)
257 264 }
258 if !shaRE.MatchString(body.Head) {
265 if !shaRE.MatchString(m.Head) {
259 266 return a, fmt.Sprintf("%s: head must be the full commit id that was reviewed", path)
260 267 }
261 a.Head = body.Head
262 if !branchNameRE.MatchString(body.Base) {
268 a.Head = m.Head
269 if !branchNameRE.MatchString(m.Base) {
263 270 return a, fmt.Sprintf("%s: base must name the branch you approved (e.g. \"main\")", path)
264 271 }
265 a.Base = body.Base
266 t, err := time.Parse(time.RFC3339, body.Created)
272 a.Base = m.Base
273 t, err := time.Parse(time.RFC3339, m.Created)
267 274 if err != nil {
268 275 return a, fmt.Sprintf("%s: created must be RFC3339: %v", path, err)
269 276 }