ci: run root vp check and drop persisted credentials (BON-18, BON-19) - #53
Conversation
Root vp check, the command in the CLAUDE.md review checklist, failed on main because apps/docs/components/logo.tsx was never formatted. The two other files BON-18 listed were fixed by later PRs. CI never caught it: the per-app check jobs run tsc, and only the bones package runs vp check. A new "Repo: Format & Lint" job runs vp check from the root, so format and lint cover every package and app. The per-app tsc jobs stay. Every checkout step in ci.yml now sets persist-credentials: false. No job there runs authenticated git after checkout. release.yml keeps the default because changesets/action pushes the version branch and the release step pushes the tag. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe CI workflow adds a workspace format and lint check, disables persisted checkout credentials, and reformats the docs logo SVG opening tag without changing its behavior. ChangesCI workflow updates
Logo markup formatting
Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: ⚪ Minimal · up to The PR adds root formatting and lint checks, formats the affected file, and disables persisted checkout credentials in CI while preserving release authentication behavior. No actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description explains the purpose, implementation details, scope, exceptions, and verification results. It covers the template's What/Why and Testing requirements through detailed sections, although it uses Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Closes BON-18 and BON-19.
BON-18: root
vp checkvp checkfrom the repo root is the command in the CLAUDE.md review checklist, and it failed onmainbecauseapps/docs/components/logo.tsxwas never formatted. The two other files the issue listed were fixed by later PRs. CI never caught it: the per-app check jobs runtsc --noEmit, and onlypackages/bonesrunsvp check.logo.tsxis formatted (vp check --fix).vp checkfrom the root. That covers format and lint for every package and app in about 5 s. The per-apptscjobs stay, since rootvp checkdoes not replace them. Lint uses oxlint defaults because the root has no lint config; it passes today with 0 errors and 1 warning onapps/demo/next-env.d.ts, a Next-generated file, which is left alone.Once this merges, the new job should be added to the
main: required checksruleset so a formatting slip cannot merge.BON-19:
persist-credentials: falseEvery
actions/checkout@v4step inci.ymlsetspersist-credentials: false(14 existing jobs plus the new one; the issue counted 15, the file had 14). No job inci.ymlruns authenticated git after checkout, andupload-artifactuses its own token.release.ymlkeeps the default becausechangesets/actionpushes the version branch and the release step pushes the tag.Verification
Root
vp checkexits 0 locally. CI on this PR exercises the new job and the checkout change.Summary by CodeRabbit
Chores
Style