From 015fc51c0d3687e5d29f9f59b214562f971bf8a5 Mon Sep 17 00:00:00 2001 From: Myasnikov Daniil Date: Mon, 9 Nov 2026 14:53:29 +0000 Subject: [PATCH] fix(release): name policy rules instead of internal numbers in output fold, run --defer and the run's own stderr printed "rule 101" for an original merged before its line started under the policy, and "rule 100" for backport/skip. Those numbers exist only inside the tool; the policy has six rules, so a reader looking for rule 101 finds nothing. Output now names such a rule in words, and the policy's own rules by their number. Assisted-By: LLM Signed-off-by: Myasnikov Daniil --- cmd/backport-batch/assemble.go | 2 +- cmd/backport-batch/body.go | 14 +++++++++++++- cmd/backport-batch/check.go | 2 +- cmd/backport-batch/check_test.go | 27 +++++++++++++++++++++++++++ cmd/backport-batch/defer.go | 2 +- 5 files changed, 43 insertions(+), 4 deletions(-) diff --git a/cmd/backport-batch/assemble.go b/cmd/backport-batch/assemble.go index ef26aace8..48c95428b 100644 --- a/cmd/backport-batch/assemble.go +++ b/cmd/backport-batch/assemble.go @@ -279,7 +279,7 @@ func (a *app) finish(b *batch, r *report, branch string, date time.Time, push, r } for _, rule := range left { for _, v := range r.skipped[rule] { - fmt.Fprintf(a.stderr, "skipped by rule %d: %s %s (%s)\n", rule, v.o.url(), terminalSafe(v.o.title), terminalSafe(v.reason)) + fmt.Fprintf(a.stderr, "skipped by %s: %s %s (%s)\n", ruleName(rule), v.o.url(), terminalSafe(v.o.title), terminalSafe(v.reason)) } } for _, p := range r.waiting { diff --git a/cmd/backport-batch/body.go b/cmd/backport-batch/body.go index a2d3a4c09..2f813233e 100644 --- a/cmd/backport-batch/body.go +++ b/cmd/backport-batch/body.go @@ -297,6 +297,18 @@ type report struct { shapes map[*verdict]string } +// ruleName is how output names what kept an original out: the policy's +// number for its own rules, and words for what the policy does not number. +func ruleName(rule int) string { + switch rule { + case ruleSkipLabel: + return labelSkip + case ruleBeforeStart: + return "rule 1, before the line's start" + } + return "rule " + strconv.Itoa(rule) +} + func (r *report) ruleTitle(rule int) string { switch rule { case ruleBeforeStart: @@ -417,7 +429,7 @@ func (r *report) outcome(v *verdict) string { case slices.ContainsFunc(r.waiting, func(p pick) bool { return p.v == v }): return "waiting for a maintainer: it changes " + codeSpan(workflowsDir) case v.rule != 0: - return "skipped by rule " + strconv.Itoa(v.rule) + ", " + titleSpan(v.reason) + return "skipped by " + ruleName(v.rule) + ", " + titleSpan(v.reason) } return "not picked" } diff --git a/cmd/backport-batch/check.go b/cmd/backport-batch/check.go index a565c3c45..1546a0197 100644 --- a/cmd/backport-batch/check.go +++ b/cmd/backport-batch/check.go @@ -209,7 +209,7 @@ func refuseFold(b *batch, n int, commit string) (string, error) { case v.rule == ruleSkipLabel: return fmt.Sprintf("#%d carries %s", n, labelSkip), nil case v.rule != 0: - return fmt.Sprintf("#%d is not one the batch takes: rule %d, %s", n, v.rule, v.reason), nil + return fmt.Sprintf("#%d is not one the batch takes: %s, %s", n, ruleName(v.rule), v.reason), nil case b.pr == nil: return fmt.Sprintf("no batch pull request is open on %s", b.line.branch()), nil case v.onBatch != "": diff --git a/cmd/backport-batch/check_test.go b/cmd/backport-batch/check_test.go index 5f4bf4f86..f93149f70 100644 --- a/cmd/backport-batch/check_test.go +++ b/cmd/backport-batch/check_test.go @@ -17,6 +17,7 @@ limitations under the License. package main import ( + "regexp" "strings" "testing" "unicode" @@ -359,6 +360,32 @@ func TestCheckDispositions(t *testing.T) { } } +// Output names a rule as the policy does: the numbers the tool keeps for +// what is no rule of the policy never reach a reader. +func TestOutputNamesNoInternalRule(t *testing.T) { + f := newFixture(t) + f.lines() + f.pr(5, "fix(app): five", "", map[string]string{"app/five.txt": "5\n"}) + f.pr(6, "fix(app): six", "", map[string]string{"app/six.txt": "6\n"}) + f.setPolicy("release-1.1: 2099-01-01\n") + gh := &fakeGH{labels: map[string][]int{labelInclude: {6}}} + head := openBatch(t, f, gh) + internal := regexp.MustCompile(`rule 1[0-9][0-9]`) + for _, args := range [][]string{ + {"fold", "--line", "release-1.1", "--original", "5", "--commit", head}, + {"run", "--line", "release-1.1", "--defer", "5", "--reason", "x"}, + } { + code, stdout, stderr := f.exec(gh, args...) + f.git("switch", "-q", "main") + if code == 0 || !strings.Contains(stdout+stderr, "#5 is not one the batch takes: rule 1, before the line's start, merged ") { + t.Errorf("%s = %d, want a refusal naming rule 1:\n%s%s", args[0], code, stdout, stderr) + } + if m := internal.FindString(stdout + stderr); m != "" { + t.Errorf("%s prints %q:\n%s%s", args[0], m, stdout, stderr) + } + } +} + type errString string func (e errString) Error() string { return string(e) } diff --git a/cmd/backport-batch/defer.go b/cmd/backport-batch/defer.go index ab9741b00..1f0679844 100644 --- a/cmd/backport-batch/defer.go +++ b/cmd/backport-batch/defer.go @@ -82,7 +82,7 @@ func (a *app) rebuild(b *batch, numbers []int, reason string, push bool) int { case v == nil: return a.fail(fmt.Errorf("#%d did not merge into main after %s branched", n, b.line.branch())) case v.rule != 0 && !slices.Contains(stale, v): - return a.fail(fmt.Errorf("#%d is not one the batch takes: rule %d, %s", n, v.rule, terminalSafe(v.reason))) + return a.fail(fmt.Errorf("#%d is not one the batch takes: %s, %s", n, ruleName(v.rule), terminalSafe(v.reason))) } hold[n] = reason }