Say a failed check first in the merge ask's headline, since a merge is not gated on the check and the headline is the phone's only warning (issue 1166)
mesh/check being carried to 90f7f87c2ac1 on ce3bedd584b8 | from 3483f603ae3d
mesh/delivery merged: source.main moved to novox/mesh-catalog@ce3bedd584b8
mesh/check being carried to 90f7f87c2ac1 on ce3bedd584b8 | from 3483f603ae3d
mesh/delivery merged: source.main moved to novox/mesh-catalog@ce3bedd584b8
Claude-Session: https://claude.ai/code/session_01NcbgfJ9Gum47zkFKWRaovL
This commit is contained in:
@@ -169,6 +169,16 @@ func approveCaveat(c checkSaid) string {
|
||||
return ""
|
||||
}
|
||||
|
||||
// mergeHeadline opens the ask's headline: "Merge into main: ", and "Check failed - merge into main: " once the check failed,
|
||||
// since a merge is not gated on the check and the headline is the phone's only warning (issue 1166). A passed or still
|
||||
// running check keeps the plain headline. The prefix leads, so phone's cut of a long headline never drops it.
|
||||
func mergeHeadline(c checkSaid) string {
|
||||
if c.Key == checkSaidFail {
|
||||
return "Check failed \u2014 merge into main: "
|
||||
}
|
||||
return "Merge into main: "
|
||||
}
|
||||
|
||||
// composeMergeAsk is the ask for a delivery, or for a group's members together.
|
||||
// check is the state of the check at the heads it covers (checkAt): the ask says "passed its check" only when it did, and
|
||||
// otherwise that Approve merges before the check ends, or although it failed (issue 1733).
|
||||
@@ -193,7 +203,7 @@ func composeMergeAsk(id string, d *Delivery, group string, members []*Delivery,
|
||||
pullSaid(m), m.Title, shortOf(m.Commit, 12), issuesSaid([]*Delivery{m}), words, touches))
|
||||
}
|
||||
if group == "" {
|
||||
a.Headline = phone("Merge into main: "+pullSaid(d)+" "+d.Title, asks.HeadlineLength)
|
||||
a.Headline = phone(mergeHeadline(check)+pullSaid(d)+" "+d.Title, asks.HeadlineLength)
|
||||
summary := fmt.Sprintf("%s %s%s. Approve merges it into main at %s%s; Decline leaves it unmerged.",
|
||||
pullSaid(d), d.Title, checkClause(check, " "), shortOf(d.Commit, 12), approveCaveat(check))
|
||||
if unreviewed != "" {
|
||||
@@ -210,7 +220,7 @@ func composeMergeAsk(id string, d *Delivery, group string, members []*Delivery,
|
||||
for _, m := range members {
|
||||
names = append(names, pullSaid(m))
|
||||
}
|
||||
a.Headline = phone("Merge into main: group "+group+" ("+strings.Join(names, ", ")+")", asks.HeadlineLength)
|
||||
a.Headline = phone(mergeHeadline(check)+"group "+group+" ("+strings.Join(names, ", ")+")", asks.HeadlineLength)
|
||||
caveat := approveCaveat(check)
|
||||
if caveat != "" {
|
||||
caveat = "," + caveat
|
||||
|
||||
@@ -3,6 +3,7 @@ package main
|
||||
import (
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"git.novox.be/novox/mesh-sdk/go/asks"
|
||||
sdk "git.novox.be/novox/mesh-sdk/go/delivery"
|
||||
@@ -42,6 +43,17 @@ func TestAReadyGroupWritesOneAskKeyWithTodaysWordsAndCallsNoRouter(t *testing.T)
|
||||
!strings.Contains(ask.Ask.Details, "mesh-catalog#2") || len(ask.Ask.Options) != 2 || ask.Ask.Who != asks.Operator {
|
||||
t.Fatalf("the ask is not today's merge ask: %+v", ask.Ask)
|
||||
}
|
||||
// A group's headline says a failed check first (issue 1166); a passed or a running one keeps the plain words.
|
||||
members := []*Delivery{{Repository: "novox/mesh-controller", Number: 1}, {Repository: "novox/mesh-catalog", Number: 2}}
|
||||
for _, c := range []struct{ key, head string }{
|
||||
{checkSaidFail, "Check failed \u2014 merge into main: group feat"},
|
||||
{checkSaidPass, "Merge into main: group feat"},
|
||||
{"running", "Merge into main: group feat"},
|
||||
} {
|
||||
if h := composeMergeAsk("id", members[0], "feat/g", members, nil, checkSaid{Key: c.key, Words: "w"}, time.Time{}).Headline; !strings.HasPrefix(h, c.head) {
|
||||
t.Fatalf("a %s check gives the headline %q, wanted it to start %q", c.key, h, c.head)
|
||||
}
|
||||
}
|
||||
// The act the Approve binds is the one the teller checks and the controller verifies: this ask's router id, the option, and
|
||||
// the heads as `owner/repo@<first 12 characters>` to the whole commit.
|
||||
approve, _ := ask.Ask.Option(OptionApprove)
|
||||
|
||||
@@ -80,8 +80,9 @@ func TestAnApprovedPullRequestIsMergedThroughTheMergeTool(t *testing.T) {
|
||||
after time.Duration
|
||||
asks, open int
|
||||
want, not []string
|
||||
head string // the ask's headline starts with it
|
||||
}{
|
||||
{name: "gated, passed", gated: true, state: "pass", asks: 1,
|
||||
{name: "gated, passed", gated: true, state: "pass", asks: 1, head: "Merge into main: app#7 Fix it",
|
||||
want: []string{"app#7 Fix it passed its check. Approve merges it into main at " + readyHead[:12] + ";"},
|
||||
not: []string{"before its check ends", "no verdict yet"}},
|
||||
{name: "ungated, no verdict", asks: 0, want: []string{waitsCheckBeforeAsking + "its check has no verdict yet"}, not: []string{"anyway"}},
|
||||
@@ -98,6 +99,7 @@ func TestAnApprovedPullRequestIsMergedThroughTheMergeTool(t *testing.T) {
|
||||
{name: "ungated, passed, its verdict gone", state: "pass", then: "gone", asks: 1, open: -1,
|
||||
want: []string{waitsCheckBeforeAsking + "its check has no verdict yet"}, not: []string{"anyway"}},
|
||||
{name: "ungated, failed", state: "fail", failed: "TestThing: wanted 1, got 2", asks: 1,
|
||||
head: "Check failed \u2014 merge into main: app#7 Fix it",
|
||||
want: []string{"its check failed: TestThing: wanted 1, got 2", "although its check failed"}, not: []string{"passed", "anyway"}},
|
||||
} {
|
||||
t.Run(row.name, func(t *testing.T) {
|
||||
@@ -143,6 +145,9 @@ func TestAnApprovedPullRequestIsMergedThroughTheMergeTool(t *testing.T) {
|
||||
}
|
||||
return
|
||||
}
|
||||
if h := v.op.ask(open[0]).Headline; row.head != "" && !strings.HasPrefix(h, row.head) {
|
||||
t.Fatalf("the headline is %q; wanted it to start %q", h, row.head)
|
||||
}
|
||||
e := v.op.ask(open[0]).Explanation
|
||||
for _, w := range row.want {
|
||||
if !strings.Contains(e, w) {
|
||||
|
||||
Reference in New Issue
Block a user