refactor: replace lint/fmt with unified vp check#665
refactor: replace lint/fmt with unified vp check#665fengmk2 merged 4 commits intonode-modules:masterfrom
Conversation
- Replace separate lint, fmt, fmtcheck scripts with check and check:fix - Update CI workflow to use 'vp run check' instead of separate lint/fmt steps - Remove redundant lint step from test job (already covered in typecheck job) - Simplify lint-staged config to use 'vp check --fix'
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refactors the project's code quality enforcement by replacing disparate Highlights
Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughReplaced separate lint/format steps and scripts with a unified "check" flow: CI workflow Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
commit: |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the linting and formatting scripts by replacing lint, fmt, and fmtcheck with a unified vp check command. The changes in package.json simplify the scripts and the lint-staged configuration. The overall change improves maintainability. I have one suggestion to improve the test script by removing side effects.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #665 +/- ##
=======================================
Coverage 92.77% 92.77%
=======================================
Files 10 10
Lines 747 747
Branches 233 233
=======================================
Hits 693 693
Misses 51 51
Partials 3 3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR consolidates formatting and linting into a single vp check entrypoint and updates local scripts + CI to use it, reducing duplicated steps while keeping code quality checks in the pipeline.
Changes:
- Replace
lint/fmt/fmtcheckscripts withcheckandcheck:fix. - Update CI workflow to run
vp run check(and remove separate lint/format-check steps). - Simplify
lint-stagedto runvp check --fix.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| package.json | Removes old lint/fmt scripts, adds check/check:fix, updates test and lint-staged to use vp check. |
| .github/workflows/nodejs.yml | Switches CI from separate lint/format steps to a single vp run check, and removes redundant linting from the test matrix job. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Line 61: The package.json "test" script currently runs "vp run check:fix"
which mutates files; change it so tests do not auto-fix—update the "test" script
(the value for the "test" npm script) to remove "vp run check:fix" and either
run a non-mutating lint/check command (e.g., "vp run check") or only run "vp
test run --reporter=dot"; if you still want an auto-fix step, move "vp run
check:fix" into a separate script (e.g., "fix" or a CI-only job) so "test"
remains deterministic and fails on style violations.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| vite-plus: | ||
| specifier: latest | ||
| version: 0.0.0-e32b32e5.20260224-0706(@arethetypeswrong/core@0.18.2)(@types/node@22.19.11)(typescript@5.9.3)(yaml@2.8.2) | ||
| version: 0.0.0-g61d318d2.20260227-0939(@arethetypeswrong/core@0.18.2)(@types/node@22.19.11)(typescript@5.9.3)(yaml@2.8.2) | ||
| vitest: | ||
| specifier: npm:@voidzero-dev/vite-plus-test@latest | ||
| version: '@voidzero-dev/vite-plus-test@0.0.0-e32b32e5.20260224-0706(@arethetypeswrong/core@0.18.2)(@types/node@22.19.11)(typescript@5.9.3)(yaml@2.8.2)' | ||
| version: '@voidzero-dev/vite-plus-test@0.0.0-g61d318d2.20260227-0939(@arethetypeswrong/core@0.18.2)(@types/node@22.19.11)(typescript@5.9.3)(yaml@2.8.2)' |
There was a problem hiding this comment.
pnpm-lock.yaml now contains two versions of @voidzero-dev/vite-plus-test (0.0.0-e32… is still used for the vitest override / @vitest/coverage-v8, while 0.0.0-g61… is pulled in by the updated vite-plus). This version split can cause subtle incompatibilities between the vp toolchain and the vitest/coverage packages, and also increases install size. Consider aligning everything onto the same @voidzero-dev/vite-plus-test version (e.g., re-run pnpm update/install so the vitest override resolves to the same g61… build, or pin the override to the matching version).
| @@ -61,15 +58,17 @@ | |||
| "test-tsc:cjs": "cd test/fixtures/ts && rm -rf node_modules && npm link ../../.. && vp run build", | |||
| "test-tsc:cjs:es2021": "cd test/fixtures/ts-cjs-es2021 && rm -rf node_modules && npm link ../../.. && vp run build", | |||
| "test-tsc:esm": "cd test/fixtures/ts-esm && rm -rf node_modules && npm link ../../.. && vp run build", | |||
| "test": "vp run lint -- --fix && vp test run --reporter=dot", | |||
| "test": "vp run check && vp test run --reporter=dot", | |||
| "bench": "vp test bench", | |||
| "test-keepalive": "cross-env TEST_KEEPALIVE_COUNT=50 vp test run --reporter=dot --test-timeout 180000 keep-alive-header.test.ts", | |||
| "test-node16": "node examples/httpclient.cjs && node examples/search_github.cjs && node examples/timing.cjs", | |||
| "cov": "vp test run --reporter=dot --coverage", | |||
| "ci": "vp run cov && vp run prepublishOnly && npm pack && attw --pack", | |||
| "clean": "rm -rf dist && tsc -b --clean", | |||
| "prepublishOnly": "vp run build", | |||
| "prepare": "husky" | |||
| "prepare": "husky", | |||
| "check": "vp check", | |||
| "check:fix": "vp check --fix" | |||
There was a problem hiding this comment.
This PR removes the lint/fmt scripts and replaces them with check/check:fix, but repo docs still reference the old commands (e.g. CLAUDE.md mentions pnpm run lint and pnpm run fmt). Update the documentation to reflect the new script names so contributors don’t follow broken instructions.
Replace separate
lint,fmt,fmtcheckscripts with unifiedvp checkcommand.Changes
lint,fmt,fmtcheckscripts withcheckandcheck:fixvp run checkinstead of separate lint/fmt stepsvp check --fixRef: voidzero-dev/setup-vp@9937ebf
Summary by CodeRabbit