Catch up only a pull request's merge into the trunk, not the merges made inside its branches (issue 1336)
This commit is contained in:
@@ -103,7 +103,10 @@ func readTrunkMerges(ctx context.Context, owner, repo, branch string) ([]trunkMe
|
|||||||
break
|
break
|
||||||
}
|
}
|
||||||
for _, c := range commits {
|
for _, c := range commits {
|
||||||
if c.SecondParent != "" {
|
// A merge of the trunk is a pull request's merge into it. A commit with a second parent that is none is a merge made
|
||||||
|
// inside some branch ("Merge remote-tracking branch 'origin/main' into fix/..."), which the trunk's history lists too and
|
||||||
|
// which no Approve ever covered: read as a gap it would raise a condition for nothing a person decides.
|
||||||
|
if c.SecondParent != "" && mergesIntoTrunk(c.Subject, branch) {
|
||||||
merges = append(merges, trunkMerge{Commit: c.Commit, Head: c.SecondParent, Subject: c.Subject, At: c.At})
|
merges = append(merges, trunkMerge{Commit: c.Commit, Head: c.SecondParent, Subject: c.Subject, At: c.At})
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -136,6 +139,12 @@ func pullOfSubject(subject string) (title string, number int, ok bool) {
|
|||||||
return m[1], n, true
|
return m[1], n, true
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// mergesIntoTrunk says a commit's subject is a pull request's merge into this trunk.
|
||||||
|
func mergesIntoTrunk(subject, branch string) bool {
|
||||||
|
m := mergeSubject.FindStringSubmatch(strings.TrimSpace(subject))
|
||||||
|
return m != nil && m[4] == branch
|
||||||
|
}
|
||||||
|
|
||||||
// headBranchOfSubject is the branch a merge commit's subject says was merged.
|
// headBranchOfSubject is the branch a merge commit's subject says was merged.
|
||||||
func headBranchOfSubject(subject string) string {
|
func headBranchOfSubject(subject string) string {
|
||||||
if m := mergeSubject.FindStringSubmatch(strings.TrimSpace(subject)); m != nil {
|
if m := mergeSubject.FindStringSubmatch(strings.TrimSpace(subject)); m != nil {
|
||||||
|
|||||||
@@ -34,7 +34,8 @@ func TestOnlyTheLastFiftyMergesOfATrunkAreRead(t *testing.T) {
|
|||||||
pages := map[int][]trunkCommit{}
|
pages := map[int][]trunkCommit{}
|
||||||
var all []trunkCommit // newest first: 51 merges, a plain commit between each pair
|
var all []trunkCommit // newest first: 51 merges, a plain commit between each pair
|
||||||
for i := 51; i >= 1; i-- {
|
for i := 51; i >= 1; i-- {
|
||||||
all = append(all, trunkCommit{Commit: fmt.Sprintf("merge-%02d", i), SecondParent: fmt.Sprintf("head-%02d", i)},
|
all = append(all, trunkCommit{Commit: fmt.Sprintf("merge-%02d", i), SecondParent: fmt.Sprintf("head-%02d", i),
|
||||||
|
Subject: fmt.Sprintf("Merge pull request 'change %d' (#%d) from fix/x into main", i, i)},
|
||||||
trunkCommit{Commit: fmt.Sprintf("plain-%02d", i)})
|
trunkCommit{Commit: fmt.Sprintf("plain-%02d", i)})
|
||||||
}
|
}
|
||||||
for i := 0; i < len(all); i += 50 {
|
for i := 0; i < len(all); i += 50 {
|
||||||
@@ -264,7 +265,8 @@ func TestAShortTrunkIsReadWholeAndAnEmptyOneIsNoMerges(t *testing.T) {
|
|||||||
if page > 1 {
|
if page > 1 {
|
||||||
return nil, nil
|
return nil, nil
|
||||||
}
|
}
|
||||||
return []trunkCommit{{Commit: "m2", SecondParent: "h2"}, {Commit: "p"}, {Commit: "m1", SecondParent: "h1"}}, nil
|
return []trunkCommit{{Commit: "m2", SecondParent: "h2", Subject: "Merge pull request 'two' (#2) from fix/b into main"},
|
||||||
|
{Commit: "p"}, {Commit: "m1", SecondParent: "h1", Subject: "Merge pull request 'one' (#1) from fix/a into main"}}, nil
|
||||||
}
|
}
|
||||||
got, err := readTrunkMerges(t.Context(), "novox", "x", "main")
|
got, err := readTrunkMerges(t.Context(), "novox", "x", "main")
|
||||||
if err != nil || len(got) != 2 || got[0].Commit != "m1" || got[1].Commit != "m2" {
|
if err != nil || len(got) != 2 || got[0].Commit != "m1" || got[1].Commit != "m2" {
|
||||||
@@ -292,3 +294,26 @@ func TestTheConditionOfAMergeNotHeardIsInPlainWords(t *testing.T) {
|
|||||||
t.Fatalf("the condition's words are not plain: %s\n%q\n%q\n%q", why, c.Headline, c.Explanation, c.Needs)
|
t.Fatalf("the condition's words are not plain: %s\n%q\n%q\n%q", why, c.Headline, c.Explanation, c.Needs)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// The trunk's history lists the merges made inside a pull request's branch too. On novox (2026-10-20) 15 of the 15 non-pull-request
|
||||||
|
// "gaps" of mesh-catalog and mesh-controller were such merges; only a pull request's merge into this trunk is a merge to catch up.
|
||||||
|
func TestAMergeMadeInsideABranchIsNoMergeOfTheTrunk(t *testing.T) {
|
||||||
|
was := trunkCommitsPage
|
||||||
|
t.Cleanup(func() { trunkCommitsPage = was })
|
||||||
|
trunkCommitsPage = func(_ context.Context, _, _, _ string, page int) ([]trunkCommit, error) {
|
||||||
|
if page > 1 {
|
||||||
|
return nil, nil
|
||||||
|
}
|
||||||
|
return []trunkCommit{
|
||||||
|
{Commit: "c0880c4d", SecondParent: "b9104454", Subject: "Merge pull request 'Read the grant file' (#378) from fix/1332 into main"},
|
||||||
|
{Commit: "6394a62a", SecondParent: "x1", Subject: "Merge remote-tracking branch 'origin/main' into fix/1081-withdraw"},
|
||||||
|
{Commit: "896f1d28", SecondParent: "x2", Subject: "Merge feat/agents-write-git-ssh into feat/agents-write-git-ssh-f (issue 955 F)"},
|
||||||
|
{Commit: "4d03755c", SecondParent: "x3", Subject: "Bring the branch up to main after #354 (issue 1047)"},
|
||||||
|
{Commit: "other", SecondParent: "x4", Subject: "Merge pull request 'Elsewhere' (#5) from fix/y into nox-mesh"},
|
||||||
|
}, nil
|
||||||
|
}
|
||||||
|
got, err := readTrunkMerges(t.Context(), "novox", "mesh-catalog", "main")
|
||||||
|
if err != nil || len(got) != 1 || got[0].Commit != "c0880c4d" {
|
||||||
|
t.Fatalf("read %+v %v: only keycloak's pull request merge into main is a merge to catch up", got, err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user