Conversation
Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
prk-Jr
left a comment
There was a problem hiding this comment.
Review: approve with nits
Verified the gate empirically in a scratch worktree at the PR head (71264e87):
- The gate has teeth. Run against
main's root.mdtree it exits 1 (AGENTS.md,CLAUDE.md,FAQ_POC.md,ProjectGovernance.mdwarned); run against this PR's tree it exits 0 (All matched files use Prettier code style!). - Correct binary.
docs/node_modules/.bin/prettieris the lockfile-pinned 3.8.1 (docs/package-lock.json), notnpx— which in this repo resolves a different version and produces false format failures. The step also sits afternpm ciin the same job, so the binary exists, andworking-directory: .resolving to the workspace root matches the in-file precedent of the "Retrieve Node.js version" step. - Glob coverage is complete. Copying all 7 regular root
.mdfiles plus a realCLAUDE.md -> AGENTS.mdsymlink into a scratch dir and appending a violation to each regular file warns on exactly 7 files. SoREADME.md,CONTRIBUTING.md,CHANGELOG.mdandTESTING.mdare covered by the glob and were already compliant; nothing that should be covered is missed.CLAUDE.mdis correctly skipped (prettier does not follow the symlink), soAGENTS.mdisn't checked twice — and the dedicated symlink-guard job runs first anyway. - No ignore-file interference. No root
.prettierignore;docs/.prettierignoreisn't on the cwd path for this invocation;.gitignorehas no.mdentries. - Content changes are formatting-only.
proseWrap: "preserve"means no reflow:AGENTS.mdis table realignment plus the new checklist entry,ProjectGovernance.mdis*→-bullet markers,FAQ_POC.mdis trailing-whitespace and double-space collapse. No semantic drift.
Nothing blocking. Four non-blocking notes inline, plus two things outside the diff:
*.mdis non-recursive, which matches the PR title, but it leaves 19 tracked Markdown files ungated (.github/pull_request_template.md, four crateREADME.mds, 14 files under.claude/). Fine to defer — worth an issue if the intent is eventually "all Markdown is formatted".- Is
format-docsa required status check? I couldn't read branch protection. If it isn't required, this gate is advisory only.
Both inline suggestions were applied in isolation and as a batch against a scratch worktree and re-verified green (exit 0, no drift), then reverted.
Document the --write fix in the CI checklist, run the root Markdown check after the VitePress dead-link build so both failures surface in one CI round, and fix a couple of long-standing typos in FAQ_POC.md touched by the same line. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
….com/IABTechLab/trusted-server into chore/check-root-markdown-formatting
ChristianPavilonis
left a comment
There was a problem hiding this comment.
Summary
Reviewed 1851e98101e68c17d900506c2ad0ac145dd6dbb7 against 6cae7f5da8911c746cf873581885f90c3820dd96. No actionable issues found.
The new root-Markdown check passed in the exact PR workflow. A separate negative test with the lockfile-pinned Prettier 3.8.1 rejected deliberately malformed copies of all seven regular root .md files, while CLAUDE.md remained a symlink to AGENTS.md. All reported CI checks pass, and the active main ruleset requires format-docs.
Summary
format-docsCI job to check root-level Markdown (AGENTS.md,README.md,FAQ_POC.md, etc.) with Prettier, closing the gap where onlydocs/was coveredAGENTS.md,FAQ_POC.md,ProjectGovernance.md) — whitespace/list-marker only, no content changesChanges
.github/workflows/format.ymlformat-docsjob that runsdocs/node_modules/.bin/prettier --config docs/.prettierrc --check "*.md"from the repo root, reusing the already-installed binary and the existingdocs/.prettierrcconfigAGENTS.mdFAQ_POC.mdProjectGovernance.mdCloses #1093
Test plan
cd docs && npm run formatdocs/node_modules/.bin/prettier --config docs/.prettierrc --check "*.md"(run from repo root)format-docsandcheck-claude-md-symlinkGitHub Actions jobs locally viaact— both pass, including the new stepCLAUDE.mdstill resolves as a symlink toAGENTS.md(git ls-treemode120000) after formattingAGENTS.mdcargo test-fastly && cargo test-axumcargo clippy-fastly && cargo clippy-axumcargo fmt --all -- --checkcd crates/trusted-server-js/lib && npx vitest runcd crates/trusted-server-js/lib && npm run formatcargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1fastly compute serveChecklist
unwrap()in production code — useexpect("should ...")tracingmacros (notprintln!)