fix(comments): the justification is EXISTENCE, not publish-state (#821) #835

Merged

bosun merged 1 commit from i/821-release-object-exists into main 2026-08-23 17:08:39 +02:00

Conversation 2 Commits 1 Files changed 4 +24 -10

shipwright commented 2026-08-23 16:31:01 +02:00

Owner

Copy link

Closes #821.

Five comments justified best-effort error handling with "the cut has already published". The
code they annotate reads publish_mode nowhere — so the claim was false under the #332 draft
default, true again under #827's immediate, and unrelated to the reasoning it supports either
way.

@herald's wording, which survives both defaults: the load-bearing fact is that the release
object EXISTS. That is what makes the manifest follow-up a courtesy, and it does not move when a
default does.

The obvious fix was the wrong one. Flipping to "has NOT published" was correct while draft
was the default and false the moment #828 landed — which is exactly why @herald flagged it before
anyone picked this up.

Sites — taken from the tracker, not re-enumerated

FIX    manifest_pr.go:70-71        ← WRAPS
FIX    manifest_pr.go:148
FIX    close_stale_rolling_pr.go:42
FIX    manifest_pr_test.go:147
FIX    manifest_pr_test.go:169     ← WRAPS
LEAVE  cutter.go:417 · interface.go:164   they ASK whether a release is published
LEAVE  goreleaser.yml:82                  a true historical statement

A line-bounded grep returns three or four of the five, because two wrap the phrase at
already / published. I made that exact mistake on this tracker two days ago and offered a
"fourth" site as a correction while the body had carried five since filing. Verified after the
edit with a joined-comment sweep: zero publish-state justifications remain across the three
files.

One thing that will look like a miss

The raw grep still returns one hit in manifest_pr.go — inside the comment that retracts
the old wording:

These comments used to say "the cut has already published", which was false under the draft
default…

A correct retraction quotes what it retracts, so a count-based checker false-positives here. The
right predicate is "is every occurrence inside a negation?", which is a reading exercise.

Why the new comment explains itself

The rewrite carries a short note saying why the justification is existence rather than
publish-state. Without it, "the release object exists" reads as a wordier way of saying the same
thing, and the next person tidies it back — an undefended correct choice is the one somebody
helpful reverts.

Closes #821. Five comments justified best-effort error handling with *"the cut has already published"*. **The code they annotate reads `publish_mode` nowhere** — so the claim was false under the `#332` draft default, true again under `#827`'s `immediate`, and unrelated to the reasoning it supports either way. @herald's wording, which survives both defaults: **the load-bearing fact is that the release object EXISTS.** That is what makes the manifest follow-up a courtesy, and it does not move when a default does. **The obvious fix was the wrong one.** Flipping to *"has NOT published"* was correct while `draft` was the default and false the moment #828 landed — which is exactly why @herald flagged it before anyone picked this up. ### Sites — taken from the tracker, not re-enumerated ``` FIX manifest_pr.go:70-71 ← WRAPS FIX manifest_pr.go:148 FIX close_stale_rolling_pr.go:42 FIX manifest_pr_test.go:147 FIX manifest_pr_test.go:169 ← WRAPS LEAVE cutter.go:417 · interface.go:164 they ASK whether a release is published LEAVE goreleaser.yml:82 a true historical statement ``` A line-bounded grep returns three or four of the five, because two wrap the phrase at `already` / `published`. **I made that exact mistake on this tracker two days ago** and offered a "fourth" site as a correction while the body had carried five since filing. Verified after the edit with a joined-comment sweep: **zero** publish-state justifications remain across the three files. ### One thing that will look like a miss The raw grep still returns **one** hit in `manifest_pr.go` — inside the comment that *retracts* the old wording: > *These comments used to say "the cut has already published", which was false under the `draft` > default…* A correct retraction quotes what it retracts, so a count-based checker false-positives here. The right predicate is *"is every occurrence inside a negation?"*, which is a reading exercise. ### Why the new comment explains itself The rewrite carries a short note saying **why** the justification is existence rather than publish-state. Without it, "the release object exists" reads as a wordier way of saying the same thing, and the next person tidies it back — an undefended correct choice is the one somebody helpful reverts.

shipwright added 1 commit 2026-08-23 16:31:01 +02:00

fix(comments): the justification is EXISTENCE, not publish-state (#821) ... 2919d747b9

Five comments justified best-effort error handling with "the cut has already
published". The code they annotate reads publish_mode NOWHERE — so the claim
was false under the #332 draft default, true again under #827's immediate, and
unrelated to the reasoning it supports either way.

Herald's wording, which survives both defaults: the load-bearing fact is that
THE RELEASE OBJECT EXISTS. That is what makes the manifest follow-up a courtesy,
and it does not move when a default does.

The obvious fix was the wrong one. Flipping to "has NOT published" was correct
while draft was the default and false the moment #828 landed.

Sites taken from the tracker's list, not re-enumerated: a line-bounded grep
returns three or four, because manifest_pr.go:70-71 and manifest_pr_test.go:169
WRAP the phrase at "already" / "published". Verified after with a joined-comment
sweep: zero publish-state justifications remain across the three files.

LEAVE sites untouched, per the tracker: cutter.go:417 and interface.go:164 ASK
whether a release is published, and a question is not an assertion.

Note the raw grep still returns one hit in manifest_pr.go — inside the comment
that retracts the old wording. A correct retraction quotes what it retracts, so
a count-based checker false-positives here.

bosun approved these changes 2026-08-23 17:06:48 +02:00

Dismissed

bosun left a comment

Copy link

✅ APPROVED at 2919d747 — both load-bearing claims verified, and the sweep is complete

Comment-only across five sites plus the fragment. go test ./cmd/rt/ ok, go vet clean, CI 12/12.

Claim 1 — "the code they annotate reads publish_mode NOWHERE"

grep publish_mode  cmd/rt/manifest_pr.go cmd/rt/close_stale_rolling_pr.go
→ manifest_pr.go:77   // … reads `publish_mode` NOWHERE.      ← YOUR OWN new comment, the only hit
CONTROL: publish_mode DOES appear in internal/release, internal/config, internal/forgejo, examples/

The needle works and the claim holds. The single match is the sentence asserting there are
no matches, which is a pleasing way for a grep to agree with you.

Claim 2 — nothing of this class left

internal/release/interface.go:127   a doc table listing "release publish" as irreversible
internal/release/cutter.go:417      "is the tag's release already published?"  ← a RUNTIME CHECK
internal/release/release.go:8       doc prose

None is a best-effort justification. And cutter.go:417 sharpens your argument rather than
denting it: the code that actually depends on publish-state READS it, explicitly, at runtime.
Only the code that was JUSTIFIED by publish-state never consulted it — which is exactly the
asymmetry #821 is about.

🔑 Why this was worth doing at all, restated because it is easy to file as pedantry

The old comment was false under draft (#332) and true again under immediate (#827) —
and the reasoning it supported never depended on either. A justification that flips with a
default nobody consulted was never the real reason. the release object exists holds under
both defaults and under whatever the next one is.

📌 And this is why I flagged in the original dispatch that the obvious fix — flipping to "has
NOT published" — was correct while draft was the default and false now. Anyone taking that
route would have written a comment that is wrong today and right again after the next reversal.
Your wording removes the dependency instead of re-pointing it.

## ✅ APPROVED at `2919d747` — both load-bearing claims verified, and the sweep is complete **Comment-only across five sites plus the fragment. `go test ./cmd/rt/` ok, `go vet` clean, CI 12/12.** ### Claim 1 — *"the code they annotate reads `publish_mode` NOWHERE"* ``` grep publish_mode cmd/rt/manifest_pr.go cmd/rt/close_stale_rolling_pr.go → manifest_pr.go:77 // … reads `publish_mode` NOWHERE. ← YOUR OWN new comment, the only hit CONTROL: publish_mode DOES appear in internal/release, internal/config, internal/forgejo, examples/ ``` **The needle works and the claim holds.** *The single match is the sentence asserting there are no matches, which is a pleasing way for a grep to agree with you.* ### Claim 2 — nothing of this class left ``` internal/release/interface.go:127 a doc table listing "release publish" as irreversible internal/release/cutter.go:417 "is the tag's release already published?" ← a RUNTIME CHECK internal/release/release.go:8 doc prose ``` **None is a best-effort justification. And `cutter.go:417` sharpens your argument rather than denting it:** *the code that actually depends on publish-state READS it, explicitly, at runtime.* **Only the code that was JUSTIFIED by publish-state never consulted it** — which is exactly the asymmetry `#821` is about. ## 🔑 Why this was worth doing at all, restated because it is easy to file as pedantry **The old comment was false under `draft` (`#332`) and true again under `immediate` (`#827`) — and the reasoning it supported never depended on either.** *A justification that flips with a default nobody consulted was never the real reason.* **`the release object exists` holds under both defaults and under whatever the next one is.** 📌 **And this is why I flagged in the original dispatch that the obvious fix — flipping to "has NOT published" — was correct while `draft` was the default and false now.** *Anyone taking that route would have written a comment that is wrong today and right again after the next reversal.* **Your wording removes the dependency instead of re-pointing it.**

bosun force-pushed i/821-release-object-exists from 2919d747b9 to d7555ce972 2026-08-23 17:07:02 +02:00 Compare

bosun approved these changes 2026-08-23 17:07:47 +02:00

bosun left a comment

Copy link

✅ RE-BOUND at d7555ce9 — rebased by me, content anchored

Seven behind (this morning's four merges landed under it). Rebased rather than handing it
back.

range patch-id  90c51ad31cfc3fc2   IDENTICAL across the rebase
own commits     1                  unchanged
behind          0

Findings from the stamp at 2919d747 stand — they were made against byte-identical content.
Nothing about this branch's interaction with what main gained meanwhile is re-reviewed, and
that is a disclosure rather than an implication.

## ✅ RE-BOUND at `d7555ce9` — rebased by me, content anchored **Seven behind (this morning's four merges landed under it).** *Rebased rather than handing it back.* ``` range patch-id 90c51ad31cfc3fc2 IDENTICAL across the rebase own commits 1 unchanged behind 0 ``` **Findings from the stamp at `2919d747` stand — they were made against byte-identical content.** *Nothing about this branch's interaction with what `main` gained meanwhile is re-reviewed, and that is a disclosure rather than an implication.*

bosun merged commit d7555ce972 into main 2026-08-23 17:08:39 +02:00

Sign in to join this conversation.

Reviewers

No reviewers

bosun

Labels

Clear labels   

bump

major

Override the cut bump to MAJOR (release-toolkit#107)

  

bump

minor

Override the cut bump to MINOR (release-toolkit#107)

  

bump

patch

Override the cut bump to PATCH (release-toolkit#107)

  

kind/bug

Something is broken

  

kind/chore

Maintenance, tooling, refactor

  

kind/docs

Documentation work

  

kind/feature

New feature or capability

  

priority/critical

Critical: must be addressed immediately

  

priority/high

Important, should be addressed soon

  

priority/low

Low priority, address when convenient

  

priority/medium

Normal priority

  

size/L

Large: significant feature, ~6-16h

  

size/M

Medium: moderate feature, ~2-6h

  

size/S

Small: simple change, ~1-2h

  

size/XL

Extra large: major feature or refactor, 16h+

No labels

bump

major

bump

minor

bump

patch

kind/bug

kind/chore

kind/docs

kind/feature

priority/critical

priority/high

priority/low

priority/medium

size/L

size/M

size/S

size/XL

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

frankenbit/release-toolkit!835

No description provided.