-
Notifications
You must be signed in to change notification settings - Fork 4
refactor: validate specifiers and rewrite policy. #103
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #103 +/- ##
==========================================
+ Coverage 93.02% 93.62% +0.59%
==========================================
Files 4 4
Lines 2208 2195 -13
==========================================
+ Hits 2054 2055 +1
+ Misses 154 140 -14 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR refactors the validation of specifiers and rewrites the rewrite policy logic to make --validate-specifiers policy-derived by default, removing explicit boolean defaults and inferring the setting from --rewrite-policy (enabled for safe and warn, disabled for skip). The changes also add validation to prevent disabling --validate-specifiers when --rewrite-policy is set to safe.
Key changes:
- Removed explicit
validateSpecifiers: truedefaults and made it policy-derived (on forsafe|warn, off forskip) - Added validation to prevent
--no-validate-specifierswhen--rewrite-policyissafe - Added comprehensive unit tests for new validation behaviors
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/init.js | Adds validation logic to prevent disabling specifiers when policy is safe; removes default value for validateSpecifiers option; updates CLI description |
| src/resolver.js | Updates validateSpecifiers to be policy-derived (defaults based on rewritePolicy instead of explicit false) |
| test/unit.js | Adds new test cases for policy validation, help output, invalid options, and missing configuration scenarios; imports init function |
| test/rewritePolicy.js | Removes explicit validateSpecifiers option from test configurations (now policy-derived) |
| test/integration.js | Skips integration tests temporarily; contains a critical bug in line 97 |
| docs/v4-migration.md | Updates documentation to reflect policy-derived validateSpecifiers behavior and adds hazard allowlist flag |
| docs/faq.md | Improves heading hierarchy by converting sub-questions to h3 headings |
| package.json | Version bump to 4.0.0-rc.7 |
| package-lock.json | Version bump to 4.0.0-rc.7 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 10 out of 11 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
No description provided.