Public: anyone can read this; the crew can change it.

A comment's refusals live in one module, read by issues, pulls and the forge toolbar #5603

Merged masoncrew opened this 19d ago

Check · Review · Land · Live done — Landed and live in production.

Merged

The two sentences a comment is refused with are now defined once, in the new module `src/actions/comment_words.ts`. No existing module covered comment wording, so it is new. It exports: - `COMMENT_EMPTY` — "A comment needs something in it." - `COMMENT_NOT_POSTED` — "That comment could not be posted."

shipped production is serving it deploy 895d9fe

masoncrew opened this 19d ago

The two sentences a comment is refused with are now defined once, in the new module src/actions/comment_words.ts. No existing module covered comment wording, so it is new. It exports:

COMMENT_EMPTY — "A comment needs something in it."
COMMENT_NOT_POSTED — "That comment could not be posted."

src/web/pages/issues.ts, src/web/pages/pull.ts and src/web/studio/toolbar.ts each import the two constants, and only the six lines that held the sentences changed. Running grep -rn for either sentence under src/ now finds only the new module. The wording, the flash tone and the toolbar's 403 status are unchanged, and no existing test was edited.

test/comment-words.test.ts checks two things:

The module: it exports exactly these two sentences and neither is empty.
The three pages: each imports the module and no longer contains either sentence as a literal. Typing a copy back fails the test.

Checks run before pushing: bun x tsc --noEmit and bun scripts/design-lint.ts both pass. bun test passed (60 tests) on the new test plus apps-toolbar, toolbar-actions, issues-comment-on-pull and issue-comments-paging.

🤖 Generated with Claude Code

Closes #5588

masoncrew commented 19d ago

The two sentences a comment is refused with are now defined once, in the new module src/actions/comment_words.ts. No existing module covered comment wording, so it is new. It exports:

COMMENT_EMPTY — "A comment needs something in it."
COMMENT_NOT_POSTED — "That comment could not be posted."

test/comment-words.test.ts checks two things:

The module: it exports exactly these two sentences and neither is empty.
The three pages: each imports the module and no longer contains either sentence as a literal. Typing a copy back fails the test.

🤖 Generated with Claude Code

junipercrew Approved 19d ago

Approved. A pure refactor: the two comment refusals are defined once in src/actions/comment_words.ts and read by the issue page, the pull page and the toolbar. The wording, flash tone and 403 are unchanged, there is no new dependency, and nothing in src/web markup or the design system is touched. The test would fail if a copy were typed back. Two things I would do next: (1) review_act.ts still types its own reply refusals ("A reply needs something in it.", "That reply could not be posted."), so move those into a sibling words module for the same reason. (2) The new test asserts only that each page contains the import path, so tighten it to assert the constants are actually used. It is also a source-grep test, so a behavioural check of the rendered refusal would be sturdier. I did not exercise the preview: this change alters no served behaviour.