From 21e453aa5fe409f141a41b43d6d23be58d6a8462 Mon Sep 17 00:00:00 2001 From: jochen Date: Tue, 13 Oct 2026 19:25:05 +0200 Subject: [PATCH] A settings proposal's summary says the layer is sent at once by a walk (ADR 0344 with ADR 0345) Rebased on #305: Approve now sends the layer by a walk, so the summary's "takes it at its next push" was false. The summary says it is set and sent at once by a walk to where it reaches, one machine at a time, each judged; the rest of what Approve does (onApprove) leads the Details, beside every key whole. --- cmd/mesh-controller/proposals.go | 18 ++++++++---------- cmd/mesh-controller/proposals_test.go | 6 +++--- cmd/mesh-controller/settings_walk_test.go | 9 ++++++--- 3 files changed, 17 insertions(+), 16 deletions(-) diff --git a/cmd/mesh-controller/proposals.go b/cmd/mesh-controller/proposals.go index 318feb16..c8aeefcc 100644 --- a/cmd/mesh-controller/proposals.go +++ b/cmd/mesh-controller/proposals.go @@ -177,6 +177,7 @@ func (p settingsProposal) ask(id string, machines []string) (asks.Ask, map[strin details := func(lines []string) string { var d strings.Builder d.WriteString(strings.Join(lines, "\n") + "\n") + d.WriteString(p.onApprove() + "\n") fmt.Fprintf(&d, "Proposed by %s at %s. Fingerprint %s.\n", sayable(p.From, machines), p.At.Local().Format("15:04 on 2 Jan"), fingerprint(p.Digest)) if !shownWhole { @@ -214,19 +215,16 @@ func (p settingsProposal) ask(id string, machines []string) (asks.Ask, map[strin return q, options } -// summary is the proposal's first line, what changes where: how many settings, how many stay, and that the machine -// takes the layer at its next push. +// summary is the proposal's first line, what changes where: how many settings, how many stay, and that Approve sends +// the layer at once by a walk, one machine at a time (novox/hq ADR 0344); the rest of what Approve does +// (onApprove) is the Details'. func (p settingsProposal) summary() string { - takes := p.Node - if p.Node == "" { - takes = "every machine" - } if p.Clear { if !p.HadLayer { return "There is no layer to remove; approving changes nothing." } - return fmt.Sprintf("Removes the %d setting(s) of %s set on %s; approved, the layer goes at once and %s takes it at its next push.", - len(leaves(p.Before, "")), p.Module, p.where(), takes) + return fmt.Sprintf("Removes the %d setting(s) of %s set on %s; approved, the layer is removed and that is sent at once "+ + "by a walk to %s, one machine at a time, each judged.", len(leaves(p.Before, "")), p.Module, p.where(), p.reaches()) } added, changed, removed := settingsChange(p.Before, p.Values) n := len(added) + len(changed) + len(removed) @@ -238,8 +236,8 @@ func (p settingsProposal) summary() string { if unchanged > 0 { stay = fmt.Sprintf(", and %d stay as they are", unchanged) } - return fmt.Sprintf("Changes %d setting(s) of %s on %s%s; approved, the layer is set at once and %s takes it at its next push.", - n, p.Module, p.where(), stay, takes) + return fmt.Sprintf("Changes %d setting(s) of %s on %s%s; approved, the layer is set and sent at once by a walk to %s, "+ + "one machine at a time, each judged.", n, p.Module, p.where(), stay, p.reaches()) } // change is what changes: as at most asks.MostPoints points for the glance, as every line whole for the note, and diff --git a/cmd/mesh-controller/proposals_test.go b/cmd/mesh-controller/proposals_test.go index 2ef2fde5..99a9bd7c 100644 --- a/cmd/mesh-controller/proposals_test.go +++ b/cmd/mesh-controller/proposals_test.go @@ -119,7 +119,7 @@ func TestAProposalAsksAtTheLevelApproveWithTheExactChange(t *testing.T) { } // The message reads at a glance (issues 383 and 639): a summary, then one point per key — and nothing else: // who proposed it, the fingerprint and the how-to are the Details, which the router notes after the answers. - summary := "Changes 3 setting(s) of mounts on shanks; approved, the layer is set at once and shanks takes it at its next push." + summary := "Changes 3 setting(s) of mounts on shanks; approved, the layer is set and sent at once by a walk to shanks, one machine at a time, each judged." if q.Explanation != summary+"\n+ sources: recalbox=‹address›@‹path›:ro\n~ shares: library=‹path› (was: none)\n- old (was: x)" { t.Errorf("the explanation:\n%s", q.Explanation) } @@ -196,7 +196,7 @@ func TestAMeshWideLayerIsProposed(t *testing.T) { } q := r.askSent(t) if q.Headline != "Set notes on the whole mesh?" || q.About != "settings.notes.mesh" || - q.Explanation != "Changes 1 setting(s) of notes on the whole mesh; approved, the layer is set at once and every machine takes it at its next push.\n+ x: 1" || + q.Explanation != "Changes 1 setting(s) of notes on the whole mesh; approved, the layer is set and sent at once by a walk to every machine running notes, one machine at a time, each judged.\n+ x: 1" || q.Whole != "" { t.Errorf("%+v", q) } @@ -316,7 +316,7 @@ func TestAClearIsProposedAndShowsWhatItRemoves(t *testing.T) { q := r.askSent(t) if q.Headline != "Clear notes on laptop?" || !strings.Contains(q.Explanation, "- places.data.owner: 1001:1001") || !strings.Contains(q.Explanation, "- places.data.path: ‹path›") || !strings.Contains(q.Whole, "- places.data.path: /srv/notes") || - !strings.HasPrefix(q.Explanation, "Removes the 2 setting(s) of notes set on laptop; approved, the layer goes at once") { + !strings.HasPrefix(q.Explanation, "Removes the 2 setting(s) of notes set on laptop; approved, the layer is removed and that is sent at once by a walk to laptop, one machine at a time") { t.Errorf("%+v", q) } if p := r.theProposal(t).Proposal; !p.Clear || p.Digest != layerDigest(r.before) || len(r.judged) != 0 { diff --git a/cmd/mesh-controller/settings_walk_test.go b/cmd/mesh-controller/settings_walk_test.go index f8cca40a..48c34f2e 100644 --- a/cmd/mesh-controller/settings_walk_test.go +++ b/cmd/mesh-controller/settings_walk_test.go @@ -82,8 +82,11 @@ func TestTheAskSaysAnApprovedLayerIsSentAtOnce(t *testing.T) { if err := q.Check(r.now); err != nil { t.Fatalf("the ask is refused: %v", err) } - if !strings.Contains(q.Explanation, "and sent at once to "+c.where+", one machine at a time, each judged, "+ - "every build there kept; its end is told as every walk's is") { + // At a glance (ADR 0345): the summary says it is sent at once by a walk, one machine at a time; the rest of + // what Approve does is the Details', which the router notes after the answers. + if !strings.Contains(q.Explanation, "sent at once by a walk to "+c.where+", one machine at a time, each judged") || + !strings.Contains(q.Details, "and sent at once to "+c.where+", one machine at a time, each judged, "+ + "every build there kept; its end is told as every walk's is") { t.Errorf("the ask does not say the layer is sent at once to %s:\n%s", c.where, q.Explanation) } approve := q.Options[0] @@ -93,7 +96,7 @@ func TestTheAskSaysAnApprovedLayerIsSentAtOnce(t *testing.T) { if !strings.Contains(words, "a walk sends it to "+c.where+" at once") { t.Errorf("the proposer's answer:\n%s", words) } - for _, said := range []string{q.Explanation, approve.Does, words} { + for _, said := range []string{q.Explanation, q.Whole, approve.Does, words} { if strings.Contains(said, "push") { t.Errorf("a push is still named, as if a second act were needed:\n%s", said) }