Drop the MergeBox duplicate mergeability headline (Forgejo #592) #593

Merged
crueber merged 1 commit from fix/issue-592 into main 2026-09-15 19:38:02 +00:00
Owner

Removes the MergeBox disp() state headline so "Able to be Merged" renders once (sidebar Mergeability value, kept). Removal-only: disp() memo, helper import, and the now-unused zeroChecks prop + call-site arg go; sidebar headline, amber blocking-reasons line, tooltip, mergeState, enabled() gate, strategy/buttons, and the mergeabilityDisplay helper itself are untouched (no fork, no helper change). Tests: new mergebox-headline-592.test.js + #592-scoped updates to the #588 MergeBox-renders pin (now an absence pin) and the pr-structure-531 MergeBox-value pin; full-minus-smoke 1486/1486 green; vite + esbuild green; go vet clean. docs/go/12_web_ui.md amended (law 12). 390px holds by construction (strictly less sidebar content).

Removes the MergeBox disp() state headline so "Able to be Merged" renders once (sidebar Mergeability value, kept). Removal-only: disp() memo, helper import, and the now-unused zeroChecks prop + call-site arg go; sidebar headline, amber blocking-reasons line, tooltip, mergeState, enabled() gate, strategy/buttons, and the mergeabilityDisplay helper itself are untouched (no fork, no helper change). Tests: new mergebox-headline-592.test.js + #592-scoped updates to the #588 MergeBox-renders pin (now an absence pin) and the pr-structure-531 MergeBox-value pin; full-minus-smoke 1486/1486 green; vite + esbuild green; go vet clean. docs/go/12_web_ui.md amended (law 12). 390px holds by construction (strictly less sidebar content).
The #588 mergeabilityDisplay mapping rendered twice: the sidebar
Mergeability value (kept, the ONE headline) and the MergeBox disp()
state line (removed with its memo, helper import, and the now-unused
zeroChecks prop + call-site arg). Sidebar, amber blocking-reasons
line, tooltip, mergeState, enabled() gate, and strategy/buttons
untouched; helper unchanged (no fork). 12_web_ui.md amended (law 12).
Author
Owner

APPROVED — independent review of fix/issue-592 (f553c72) vs #592 acceptance. All criteria hold, no fix commits needed.

Findings (verified in /tmp/walhub-592, diff origin/main..origin/fix/issue-592):

  • MERGE headline gone, nothing else: MergeBox diff removes only the mergeabilityDisplay import, the disp() memo (+ its mergeable/checksBlockers/reviewDecision/zeroChecks detail), and the disp().cls/text + disp().sub

    /Show lines. Reasons line (blocking merge: blockers().join), tooltip (machine wording), mergeState arms (dirty, clean/behind), enabled() gate (mergeable+canMerge+!merging), strategy select, merge/update-branch buttons all byte-identical (remaining diff lines are the memo body + Show close only).

  • zeroChecks removal complete: no zeroChecks in MergeBox.jsx; call-site arg zeroChecks={zeroChecks} gone from Pull.jsx; page-level zeroChecks() signal retained and still consumed by sidebar headline (1384) + Checks section (1424) + lib helper — correct.
  • Helper untouched, single headline: pull-state.js has zero diff; exactly one 'function mergeabilityDisplay(' def; MergeBox has zero code refs (1 hit is a comment); Pull.jsx has the single consumer mergeabilityDisplay(m?.state...) via mergeabilityView, rendered unconditionally as <p class=text-sm ${v.cls}>{v.text}

    — renders in every state incl. blocked (helper covers blocked soft-red), so sidebar is the single headline.
  • #588 pins updated not weakened: mergeability-display-588 MergeBox-renders test is now a 7-assertion absence pin (import/call/disp/text/sub/zeroChecks + call-site arg gone) while keeping green/red tone assertions through the sidebar helper; pr-structure-531 MergeBox-value pin is now reasons-kept + no-second-headline. Strictly stronger.
  • Docs (law 12): docs/go/12_web_ui.md #592 amendment present in same change. No new deps (law 1): package.json still exactly solid-js/@solidjs/router/marked/dompurify; helper change is removal-only, no new CSS (no style= in MergeBox).
  • Tests fail pre-fix: origin/main MergeBox contains disp().text (grep count 1) so the new no-headline test fails on main by construction. Post-fix: node --test minus smoke 1486/1486 green (matches PR claim); smoke excluded correctly (needs live server; local :8080 returns 403, unrelated). go vet ./... clean.

Verdict: APPROVED. No defects; nothing fixed in worktree, nothing pushed.

APPROVED — independent review of fix/issue-592 (f553c72) vs #592 acceptance. All criteria hold, no fix commits needed. Findings (verified in /tmp/walhub-592, diff origin/main..origin/fix/issue-592): - MERGE headline gone, nothing else: MergeBox diff removes only the mergeabilityDisplay import, the disp() memo (+ its mergeable/checksBlockers/reviewDecision/zeroChecks detail), and the disp().cls/text + disp().sub <p>/Show lines. Reasons line (blocking merge: blockers().join), tooltip (machine wording), mergeState arms (dirty, clean/behind), enabled() gate (mergeable+canMerge+!merging), strategy select, merge/update-branch buttons all byte-identical (remaining diff lines are the memo body + Show close only). - zeroChecks removal complete: no zeroChecks in MergeBox.jsx; call-site arg zeroChecks={zeroChecks} gone from Pull.jsx; page-level zeroChecks() signal retained and still consumed by sidebar headline (1384) + Checks section (1424) + lib helper — correct. - Helper untouched, single headline: pull-state.js has zero diff; exactly one 'function mergeabilityDisplay(' def; MergeBox has zero code refs (1 hit is a comment); Pull.jsx has the single consumer mergeabilityDisplay(m?.state...) via mergeabilityView, rendered unconditionally as <p class=text-sm ${v.cls}>{v.text}</p> — renders in every state incl. blocked (helper covers blocked soft-red), so sidebar is the single headline. - #588 pins updated not weakened: mergeability-display-588 MergeBox-renders test is now a 7-assertion absence pin (import/call/disp/text/sub/zeroChecks + call-site arg gone) while keeping green/red tone assertions through the sidebar helper; pr-structure-531 MergeBox-value pin is now reasons-kept + no-second-headline. Strictly stronger. - Docs (law 12): docs/go/12_web_ui.md #592 amendment present in same change. No new deps (law 1): package.json still exactly solid-js/@solidjs/router/marked/dompurify; helper change is removal-only, no new CSS (no style= in MergeBox). - Tests fail pre-fix: origin/main MergeBox contains disp().text (grep count 1) so the new no-headline test fails on main by construction. Post-fix: node --test minus smoke 1486/1486 green (matches PR claim); smoke excluded correctly (needs live server; local :8080 returns 403, unrelated). go vet ./... clean. Verdict: APPROVED. No defects; nothing fixed in worktree, nothing pushed.
Sign in to join this conversation.
No description provided.