Phase 6: README + planning-doc rewrites (Subscribe/Publish nomenclature)#103
Conversation
…h (livetemplate v0.10.0) Phase 6 of the broadcast-redesign wave (livetemplate#415) — examples side. go.mod bumped from livetemplate v0.9.0 → v0.10.0. Three apps migrated (landing-demo intentionally skipped per project-wide Phase-6 scope decision — the recipe is the same as the others if it needs picking up later): - chat/main.go: Mount adds ctx.Subscribe(ctx.SelfTopic()); Join/Send/Leave swap 3 BroadcastAction call sites to ctx.Publish(ctx.SelfTopic(), ...). Mount doc comment rewritten. - shared-notepad/main.go: Mount adds Subscribe; Save swaps to Publish. - shared-notepad/notepad.tmpl: "How it works" list bullet reworded. - todos/controller.go: Mount adds Subscribe; 4 call sites swap to Publish. Teaching-text rewrites: - README.md: "Real-time sync" bullet reframed to the two-step opt-in shape. - docs/plans/improve-ui-ux.md: 2 sites in the showcase planning notes. Go test suite green on chat / shared-notepad / todos. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
896024a to
b6ae50e
Compare
Code ReviewOverview: Pure teaching-text update (3 files, +4/-4) renaming the broadcast API from What's good
SuggestionsREADME.md — bullet is getting long The new text:
This is now quite a mouthful for a scannable feature list. Consider splitting the how into a sub-bullet or prose note, or shortening to something like:
The full signatures are more useful in a guide or API reference than in a README overview bullet.
This text appears directly in the running app's "How it works" card, visible to end users. Terms like "Mount-side opt-in" and "explicit Publish" are framework internals. End users benefit more from what happens, not how it's wired. The original phrasing was closer to the right level:
Framework concepts like Subscribe/Publish belong in the docs/guides, not the live app UI.
The more precise No blockersThe rename itself is accurate and the scope is minimal. The two suggestions above are about prose clarity, not correctness. The shared-notepad UI copy is the one I'd push back on most — implementation detail in end-user UI copy is a usability concern worth addressing before merge. |
Two prose-quality items from the review:
- README.md: shorten the "Real-time sync" bullet. The previous text spelled
out both call signatures inline, making the scannable feature-list bullet
unwieldy. The full signatures live in the docs/recipes; the README needs
just the gist + the pattern name.
- shared-notepad/notepad.tmpl: revert the "How it works" bullet to the
user-facing wording. The previous text ("Mount-side opt-in plus an
explicit Publish from Save…") appeared in the running app's UI card,
surfacing framework-internal terms ("Mount-side opt-in", "explicit
Publish") to end users. Framework concepts belong in the
docs/guides; the UI card should describe *what happens*, not *how it's
wired*. Restored to the pre-v0.10.0 phrasing.
The docs/plans/improve-ui-ux.md text (author-facing demo recording
script) keeps the precise `Publish(SelfTopic(), ...)` wording — bot
confirmed that level of detail is appropriate there.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Code ReviewPR: Phase 6: README + planning-doc rewrites (Subscribe/Publish nomenclature) OverviewThis PR updates teaching text to replace the old Observations
|
| Area | Status |
|---|---|
| Correctness | No functional code — nothing to break |
| Conventions | Mostly aligned; see template bullet above |
| Test coverage | N/A (doc-only) |
| Security | N/A |
The main ask is reconsidering the shared-notepad/notepad.tmpl bullet — it currently removes API visibility from the one place in this file dedicated to explaining the mechanism. The README jargon note is a softer suggestion.
🤖 Generated with Claude Code
Round-2 Claude bot reviewer noted: docs/plans/improve-ui-ux.md had two different conventions for naming the Publish call — one with the `ctx.` receiver, one without. Aligning both to `ctx.Publish(ctx.SelfTopic(), ...)` matches the README + recipe convention. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Declining the round-2 bot suggestions on
Round-2 substantive item addressed: the |
|
Code Review - Phase 6: README + planning-doc rewrites Summary: Documentation-only PR (+4/-4) that migrates teaching text from the deprecated BroadcastAction nomenclature to the new ctx.Subscribe / ctx.Publish two-step API. No Go code or test changes. Issues 1. shared-notepad/notepad.tmpl - Regression in teaching value The old label ( <li><strong>ctx.Subscribe / ctx.Publish</strong> — Save in one tab refreshes the others for the same user</li>This keeps the 'How it works' list consistent: each 2. README.md - Jargon in user-facing copy "the two-step peer-fan-out pattern" is internal framework jargon that adds length without helping a new user. The API names alone are sufficient:
3. Inconsistency between README.md and docs/plans/improve-ui-ux.md The same feature is described two different ways in this PR. The planning-doc version ( What is Good
Summary: One functional concern (the notepad template loses its teaching value by dropping the API concept name) and one style concern (jargon in the README). Both are one-line fixes. The planning-doc changes are solid as-is. Generated with Claude Code |
Two items from the round-3 review: 1. shared-notepad/notepad.tmpl — name the API in the "How it works" list, matching the adjacent BasicAuth bullet pattern. The list's other bullets (BasicAuth, Notes persist…) name framework concepts; "Real-time sync" was the outlier in describing only the behavior. Updated to "ctx.Subscribe / ctx.Publish" — the new API surface, in the format user-readable but still naming the mechanism. 2. README.md — drop the "the two-step peer-fan-out pattern" parenthetical. The bot's point: API names alone are sufficient for a feature-list bullet; "peer-fan-out" is internal framework language that adds length without helping a new reader. Bullet now reads "via opt-in ctx.Subscribe / ctx.Publish". Round 1 → 2 → 3 of bot feedback shifted positions on the notepad bullet (remove API names → re-add API names with verbose framing → name them directly). Round 3's framing (consistency with the adjacent BasicAuth naming pattern) is the strongest argument and what's landing here. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Code Review — PR 103 (Phase 6: README + planning-doc rewrites) OVERVIEW WHAT LOOKS GOOD
SUGGESTIONS (minor, non-blocking)
NO ISSUES FOUND
VERDICT |
|
Round-4 review reads as LGTM-with-optional-nits ("Changes are correct, consistent with the multi-repo rename, and touch no executable code. Both suggestions are cosmetic."). Declining both nits with rationale (per CLAUDE.md bot review convergence — two consecutive rounds of cosmetic-only feedback is the convergence signal): "opt-in" qualifier in notepad.tmpl bullet: The bullet text is constrained to ~80 chars to fit alongside "BasicAuth", "Notes persist…" in the running app's UI card. Adding "(opt-in)" pushes the line into wrap territory on narrow viewports without changing the user's information at the moment they're reading the running demo. The README is the place where "opt-in" carries weight (the reader is comparing to other peer-fan-out frameworks); the notepad UI card is descriptive, not comparative. "RefreshTodos" hardcoded in demo script: the planning-doc demo script is a step-by-step recording guide — naming the specific action method gives the recorder a concrete thing to click and verify. If the action ever gets renamed, the recording guide deserves updating with the rename anyway (the GIF the script produces shows the action name in the network tab); the bot's "generic via ctx.Publish fan-out" framing would lose the click-target precision a recording guide needs. Holding the current text. |
Phase 6 of the broadcast-redesign wave (livetemplate#415) — examples side, teaching-text-only after rebase.
Scope (post-rebase, net-new on top of origin/main)
examples#102 (already merged) covered the chat/shared-notepad/todos/landing-demo Go migrations + go.mod bump. This PR is the teaching-text companion that #102 didn't touch:
README.md— "Real-time sync" bullet reframed to the two-step opt-in shape.docs/plans/improve-ui-ux.md— 2 sites in the showcase planning notes.shared-notepad/notepad.tmpl— "How it works" list bullet.3 files, +4/-4.
Audit-was-wrong note
Same recovery as livetemplate/docs#27 — original Phase 6 worktree branched off pre-#102 SHA and didn't notice the merged work. Rebase dropped the duplicates, kept only net-new teaching text.
Companion PRs
🤖 Generated with Claude Code