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/vole.git

Write over SSH with a registered key:

git clone ssh://git@rodent.software:2222/vole.git

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

Say what each change is waiting for in the list

7cc36ba0f4ba96d3f4938b13758b2cc20c22375a · Keenan Nemetz · 5d ago · parent 946d2fd4

Say what each change is waiting for in the list

A change spends most of its life in states "open" cannot describe: held for a
maintainer to allow CI, running its checks, failed, waiting for a reviewer,
conflicting with its base. The list now says which of those it is, in one
column, with a tooltip that explains the label.

The answer comes from facts a list can afford: the approvals it already counts,
one batched run-queue query for every head on the page, the contributor
allowance (one ref listing, not one per row), and — only for a change that would
otherwise read as ready — whether it still merges into its base. The full
landing plan stays on the detail page.

The rest of the row follows the issue list: the author and opening moved under
the title, the comment count sits at the end of that line, and the last column
is "Updated", the newest commit over the request's directory. The list sorts by
it. The separate Author, Approvals and Age columns are gone.

Closes #194

server/web/change_status.go +225

@@ -0,0 +1,225 @@
1 package web
2
3 // The change list's one status column.
4 //
5 // A change is not simply open or closed: it is waiting for a maintainer to allow
6 // CI, or for jobs to finish, or for a reviewer, or for the branch to be updated.
7 // The list says which of those it is, in one word, so a reader can tell what the
8 // next move is without opening every change.
9 //
10 // The answer is derived from facts a list can afford: the approvals it already
11 // counted, the runs recorded for the head commit, whether a contributor tip is
12 // still waiting to be allowed, and — only for a change that would otherwise read
13 // as ready — whether it still merges into its base. The full landing plan belongs
14 // to the detail page; nothing here moves a ref or queues a run.
15
16 import (
17 "fmt"
18 "sort"
19 "strings"
20
21 "github.com/rodent-software/vole/ci/runs"
22 "github.com/rodent-software/vole/forge/access"
23 "github.com/rodent-software/vole/forge/repo"
24 )
25
26 // changeStatus is what the list's Status column says about one row. Key styles
27 // the label, Label is the words, Icon is the glyph beside them, and Note is the
28 // tooltip that says why.
29 type changeStatus struct {
30 Key string
31 Label string
32 Icon string
33 Note string
34 }
35
36 // changeChecks reduces the runs for one head commit to the two questions the
37 // status asks: is anything still running, and did anything fail.
38 type changeChecks struct {
39 Pending bool
40 Failed bool
41 }
42
43 // changeStatusFor picks the single status. The order is the order of the next
44 // move: a held CI run blocks everything, a failure is the author's turn, running
45 // jobs are only a wait, and review comes after the checks that inform it.
46 func changeStatusFor(state, base string, current, required int, checks changeChecks, ciHeld, conflictBase string) changeStatus {
47 switch state {
48 case "merged":
49 return changeStatus{
50 Key: "merged", Label: "Merged", Icon: "git-merge",
51 Note: "This change has landed on " + branchName(base) + ".",
52 }
53 case "closed":
54 return changeStatus{
55 Key: "closed", Label: "Closed", Icon: "circle-x",
56 Note: "This change was closed without merging.",
57 }
58 }
59 if ciHeld != "" {
60 return changeStatus{
61 Key: "waiting", Label: "Waiting on CI approval", Icon: "play",
62 Note: "CI is held until a maintainer allows " + ciHeld + " at this commit.",
63 }
64 }
65 if checks.Failed {
66 return changeStatus{
67 Key: "failed", Label: "CI failed", Icon: "circle-x",
68 Note: "A run for this commit failed. Pushing a fix starts a new review.",
69 }
70 }
71 if checks.Pending {
72 return changeStatus{
73 Key: "waiting", Label: "Waiting on CI jobs", Icon: "activity",
74 Note: "Checks for this commit are queued or running.",
75 }
76 }
77 if current < required {
78 return changeStatus{
79 Key: "review", Label: "Waiting for review", Icon: "circle-dot",
80 Note: fmt.Sprintf("%d of %d required approvals.", current, required),
81 }
82 }
83 if conflictBase != "" {
84 return changeStatus{
85 Key: "conflict", Label: "Merge conflict", Icon: "git-branch",
86 Note: "This no longer merges cleanly into " + conflictBase + ". Update the branch.",
87 }
88 }
89 note := "No approval is required and the checks on this commit passed."
90 if required > 0 {
91 note = "The required approvals are in and the checks on this commit passed."
92 }
93 return changeStatus{
94 Key: "ready", Label: "Ready to merge", Icon: "circle-check", Note: note,
95 }
96 }
97
98 // branchName keeps a note readable when a fixture or an old record names no base.
99 func branchName(base string) string {
100 if base == "" {
101 return "its base branch"
102 }
103 return base
104 }
105
106 // decorateChangeStatuses fills the Status of every row. It is called once per
107 // page, after the approval counts are known and before the template runs.
108 func (w *Web) decorateChangeStatuses(m *repo.Meta, rows []requestRow) {
109 if len(rows) == 0 {
110 return
111 }
112 open := false
113 for _, row := range rows {
114 if row.State == "open" {
115 open = true
116 break
117 }
118 }
119 held := map[string]string{}
120 if open {
121 held = w.heldContributorHeads(m)
122 }
123 checksByHead := w.changeChecks(m, rows)
124 for i := range rows {
125 row := &rows[i]
126 checks := checksByHead[row.Head]
127 ciHeld, conflictBase := "", ""
128 if row.State == "open" {
129 ciHeld = held[row.Head]
130 // A conflict only changes the answer for a change that would
131 // otherwise read as approved, so the merge is asked for that row and
132 // no other. A page of rows waiting for review costs no merges.
133 if ciHeld == "" && !checks.Failed && !checks.Pending && row.Current >= row.Required {
134 conflictBase = w.changeConflictBase(m, *row)
135 }
136 }
137 row.Status = changeStatusFor(row.State, row.Base, row.Current, row.Required, checks, ciHeld, conflictBase)
138 }
139 }
140
141 // changeChecks reads the newest run per job for every head on the page in one
142 // query. A retried job's newest attempt is the one that counts, which is what the
143 // store returns.
144 func (w *Web) changeChecks(m *repo.Meta, rows []requestRow) map[string]changeChecks {
145 out := map[string]changeChecks{}
146 if w.Runs == nil {
147 return out
148 }
149 shas := make([]string, 0, len(rows))
150 seen := map[string]bool{}
151 for _, row := range rows {
152 if row.Head == "" || seen[row.Head] {
153 continue
154 }
155 seen[row.Head] = true
156 shas = append(shas, row.Head)
157 }
158 bySHA, err := w.Runs.ForSHAs(m.Name, shas)
159 if err != nil {
160 return out
161 }
162 for sha, byKey := range bySHA {
163 var checks changeChecks
164 for _, run := range byKey {
165 switch run.Status {
166 case runs.StatusFailed, runs.StatusCanceled:
167 checks.Failed = true
168 case runs.StatusQueued, runs.StatusRunning:
169 checks.Pending = true
170 }
171 }
172 out[sha] = checks
173 }
174 return out
175 }
176
177 // heldContributorHeads names the contributor branch behind each head whose CI is
178 // still waiting for a maintainer. One ref listing answers for the whole page, and
179 // the first branch at a tip decides, which is the rule the change page uses.
180 func (w *Web) heldContributorHeads(m *repo.Meta) map[string]string {
181 held := map[string]string{}
182 if w.Runs == nil {
183 return held
184 }
185 refs, err := m.Git().ListRefs("refs/heads/")
186 if err != nil {
187 return held
188 }
189 names := make([]string, 0, len(refs))
190 for ref := range refs {
191 names = append(names, ref)
192 }
193 sort.Strings(names)
194 for _, ref := range names {
195 if _, ok := access.ContributorRefName(ref); !ok {
196 continue
197 }
198 head := refs[ref]
199 if _, seen := held[head]; seen {
200 continue
201 }
202 if allowed, err := w.Runs.CIAllowed(m.Name, ref, head); err == nil && !allowed {
203 held[head] = strings.TrimPrefix(ref, "refs/heads/")
204 }
205 }
206 return held
207 }
208
209 // changeConflictBase returns the base branch when this change no longer merges
210 // into it, and "" when it does or when the question cannot be answered. It is
211 // the same check the change page's Review section uses, asked for one row.
212 func (w *Web) changeConflictBase(m *repo.Meta, row requestRow) string {
213 if row.Base == "" || row.Head == "" {
214 return ""
215 }
216 baseTip, err := m.Git().RevParse("refs/heads/" + row.Base)
217 if err != nil || baseTip == "" {
218 return ""
219 }
220 _, conflicts, err := m.Git().MergeTree(baseTip, row.Head)
221 if err != nil || len(conflicts) == 0 {
222 return ""
223 }
224 return row.Base
225 }