chore(charlie): align next/v3 docs/playbooks with main for Raw, Conditional, tests#361
Conversation
There was a problem hiding this comment.
Overall the new .charlie instructions and playbooks are coherent and aligned with the repository’s next/v3 setup, but there are a couple of consistency and maintainability concerns. The biggest issue is that the newly added [R4] verification rules conflict with the behavior described in this PR’s own description, which skips those commands for docs-only changes. The Conditional/Raw playbook hardcodes the MSO closer string without pointing to a single source of truth, which could drift over time. The rollback command in the CI-aligned test playbook assumes a tracking origin branch and may be surprising or brittle for some contributors.
Additional notes (4)
- Readability |
.charlie/instructions/pull-request.md:29-41
TheVerification commandssection definesrepo:lintas part of R4, then introduces a separate R4.1 section for linting. While this mirrors the intent frommain, the split between runningrepo:lintas a verification command and having a separate mandatory linting rule can be slightly confusing for contributors scanning quickly (it reads as if lint must be run twice in different contexts).
Given this is a fresh copy on next/v3, consider making the relationship between R4 and R4.1 explicit (e.g., that R4.1 elaborates on the repo:lint bullet in R4 rather than adding a distinct additional requirement). This would reduce ambiguity without changing any actual rules.
- Maintainability |
.charlie/playbooks/conditional-and-raw.md:16-18
The Closer/opener integrity guidance hardcodes the closer string to<![endif]/-->. Ifnext/v3ever needs to diverge (or if older tests still assert<![endif]-->), this guidance could become inaccurate and mislead contributors writing or updating tests.
Given that #348 tracks alignment across branches, it might be safer to either:
-
briefly justify why this closer is the canonical form for
next/v3, or -
reference a single source of truth (e.g., a helper or existing tests) rather than duplicating the literal in prose.
-
Maintainability |
.charlie/playbooks/conditional-and-raw.md:13-13
The playbook clearly targetsnext/v3and mentions #348, which is good. One minor risk is that the explicit command stringimport { Conditional, Raw, render } from '../src/index.ts'may drift if the source entry point changes onnext/v3(for example, if the main export file is renamed or reorganized). That would make the guidance subtly stale.
Given this is an internal contributor playbook, you might want to phrase this as a pattern ("import from ../src/index.ts or the relevant source file") to reduce future maintenance when the source layout evolves.
- Maintainability |
.charlie/playbooks/run-jsx-email-tests-like-ci.md:29-32
The rollback command that targets snapshots uses a glob inside quotes and relies ongit restore --source=origin/$(git rev-parse --abbrev-ref HEAD)succeeding. On some shells or for contributors without the remote tracking branch locally (e.g., working from a fork or a non‑tracking branch), this may either no‑op or surprise people.
Given this is meant as a safety valve for contributors, a slightly more conservative example (or a brief warning about the assumptions) would make it easier to use without accidentally restoring from the wrong remote/branch.
Summary of changes
Summary of changes
- Added
.charlie/instructions/pull-request.mddescribing branch-specific rules for thenext/v3branch, with emphasis onpackages/jsx-email, Moon tasks, testing, linting, and PR conventions. - Introduced
.charlie/playbooks/conditional-and-raw.mdto guide changes to<Conditional>and<Raw>rendering and tests onnext/v3, including test strategies and required commands. - Added
.charlie/playbooks/run-jsx-email-tests-like-ci.mddetailing how to run thepackages/jsx-emailtests locally in a way that mirrorsnext/v3CI (build steps, Moon tasks, Vitest config, rollback guidance).
Component / Package Name:
jsx-email / .charlie
This PR contains:
Are tests included?
Breaking Changes?
List any relevant issue numbers:
Resolves #354
Refs #348
Description
Aligns the
next/v3branch docs and Charlie playbooks withmainfor<Raw>,<Conditional>, and runningjsx-emailtests so v3 contributors get branch-accurate guidance..charlie/instructions/pull-request.mdfrommainintonext/v3and updated test references to use thejsx-email:testMoon task andshared/vitest.config.ts(the configuration used onnext/v3)..charlie/playbooks/conditional-and-raw.mdmirroringmain’s Conditional/Raw guidance, but wired to thenext/v3Moon tasks and CI workflow. This copy explicitly calls out that alignment work should branch fromorigin/next/v3, targetnext/v3as the PR base, and mention#348..charlie/playbooks/run-jsx-email-tests-like-ci.mddescribing how to run thepackages/jsx-emailsuite locally in the same waynext/v3CI does (build plugin artifacts, runpnpm moon run jsx-email:test, and runjsx-email:tsc), again with an explicit note about targetingnext/v3and referencing#348for the broader alignment effort.docs/components/{raw,conditional}.mdunchanged because they are already in sync betweenmainandnext/v3; the new.charliedocs provide the branch-specific contributor guidance requested in charlie: next/v3 docs and playbooks parity with main for Raw, Conditional, and tests #354.Verification
Docs-only change (new
.charlieinstructions and playbooks); no runtime or test code was modified.pnpm moon run repo:lint,pnpm moon run jsx-email:build,pnpm moon run jsx-email:test, andpnpm moon run jsx-email:tscbecause this PR only adds documentation/configuration for Charlie and does not touch TypeScript/JS or workflows.jsx-email:test,jsx-email:tsc, pluginbuildtasks, andshared/vitest.config.ts) exist onnext/v3and match the CI workflows (.github/workflows/test*.yml).Self-review notes