♻️ Sort field & Spport CLI #296
Conversation
[skip ci] <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Expanded the list of allowed packages to include `kwxoswoff20jscss`, enabling its use in the project. - **Version Update** - Project version updated to `1.132.0`. <!-- end of auto-generated comment: release notes by coderabbit.ai --> (cherry picked from commit bc3a35a)
<!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Chores** - Updated allowed scopes in package management: removed `@scent` and added `@felce`. - No changes made to `devDependencies` or other core fields in `package.json`. <!-- end of auto-generated comment: release notes by coderabbit.ai --> (cherry picked from commit ae318f1) # Conflicts: # package.json
WalkthroughThe changes in this pull request include the removal of the Changes
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
唯一缺点是，当前 PR 会破坏 blame, 以后会不好追溯。大家都洗白了... |
[skip ci] ## [1.133.0](cnpm/unpkg-white-list@v1.132.0...v1.133.0) (2024-10-10) ### Features * add [@felce](https://github.com/felce) ([cnpm#294](cnpm#294)) ([ae318f1](cnpm@ae318f1)) * add kwxoswoff20jscss ([cnpm#295](cnpm#295)) ([bc3a35a](cnpm@bc3a35a)) (cherry picked from commit 61d6cfa)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (7)
eslint.config.mjs (1)
1-2: LGTM! Consider using named imports for clarity.The imports are correct and necessary for the ESLint configuration. However, for improved readability, consider using named imports.
You could refactor the imports as follows:
-import pluginJsonc from "eslint-plugin-jsonc"; -import parserJsonc from "jsonc-eslint-parser"; +import { configs } from "eslint-plugin-jsonc"; +import { parser as jsoncParser } from "jsonc-eslint-parser";This change would make it clearer which specific exports are being used from each package.
README.md (2)
13-23: LGTM: Comprehensive CLI usage examplesThe new section on adding specific package names and versions is well-structured and provides clear examples for different scenarios. This aligns perfectly with the PR objective of introducing CLI support.
Consider adding a brief explanation of the difference between the three examples (all versions, specific version, version range) to further clarify their use cases for users who might be less familiar with semantic versioning.
Line range hint
25-35: LGTM: Helpful JSON examplesThe added JSON examples effectively illustrate the expected changes in the
package.jsonfile after using the CLI commands. This visual representation enhances user understanding and complements the CLI instructions well.Consider adding a brief comment above each JSON example to explicitly state that these are the resulting changes in the
package.jsonfile. This would provide even more context for users who might be skimming the document.Also applies to: 45-52
scripts/generate.js (4)
1-36: LGTM! Consider enhancing the help message.The initial setup, imports, and constant definitions look good. The use of environment variables for debugging and dynamic output path generation are well-implemented.
Consider updating the help message to include information about the debug mode:
const HELP = ` Usage: npm run add --package={package name}:{version range} npm run add --scope=@{scope name} + + Debug mode: + Set DEBUG=true environment variable to use draft output and enable debug logging. `;
38-74: LGTM! Consider enhancing error handling.The
addPkgfunction is well-implemented, including input validation, existence checking, and sorting of packages. It aligns well with the PR objectives.Consider using a more consistent approach to error handling:
function addPkg(input) { if (typeof input !== 'string' || input.length === 0) { - return console.log('💥 Invalid package name'); + throw new Error('Invalid package name'); } const [name, version] = input.split(':'); + + if (!version) { + throw new Error('Invalid package format. Use {package name}:{version range}'); + } // exits if (PKG.allowPackages[name]) { throw new Error(`Package ${name} already exists`); } // ... rest of the function }This change ensures that all error cases are handled consistently by throwing errors, which can be caught and logged in the main function.
76-102: LGTM! Consider aligning error handling with addPkg function.The
addScopefunction is well-implemented, including input validation, existence checking, and sorting of scopes. It aligns well with the PR objectives.For consistency with the suggested changes in the
addPkgfunction, consider updating the error handling:function addScope(input) { if (typeof input !== 'string' || input.length === 0) { - return console.log('💥 Invalid scope name'); + throw new Error('Invalid scope name'); } else if (!input.startsWith('@')) { input = '@' + input; } DEBUG && console.log(`Add scope: ${input}`); // exits if (PKG.allowScopes.includes(input)) { throw new Error(`Scope ${input} already exists`); } // ... rest of the function }This change ensures that all error cases are handled consistently by throwing errors, which can be caught and logged in the main function.
104-132: LGTM! Consider enhancing argument parsing and error handling.The
mainfunction effectively manages the script's execution flow, including argument parsing and function calls. The implementation of debug logging is commendable.Consider the following improvements:
- Use a command-line argument parsing library like
yargsorcommanderfor more robust argument handling:const yargs = require('yargs/yargs'); const { hideBin } = require('yargs/helpers'); function main() { const argv = yargs(hideBin(process.argv)) .usage(HELP) .option('pkg', { describe: 'Add a package', type: 'string' }) .option('scope', { describe: 'Add a scope', type: 'string' }) .help('h') .alias('h', 'help') .argv; if (argv.pkg) { addPkg(argv.pkg); } else if (argv.scope) { addScope(argv.scope); } else { console.log(HELP); } }
- Implement error handling:
function main() { // ... argument parsing try { if (argv.pkg) { addPkg(argv.pkg); } else if (argv.scope) { addScope(argv.scope); } else { console.log(HELP); } } catch (error) { console.error('Error:', error.message); process.exit(1); } }These changes will make the script more robust and user-friendly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
- .eslintrc (0 hunks)
- .gitignore (1 hunks)
- README.md (2 hunks)
- eslint.config.mjs (1 hunks)
- scripts/generate.js (1 hunks)
💤 Files with no reviewable changes (1)
- .eslintrc
✅ Files skipped from review due to trivial changes (1)
- .gitignore
🧰 Additional context used
🔇 Additional comments (9)
eslint.config.mjs (4)
4-6: LGTM! Proper use of flat config format and recommended configurations.The configuration export is well-structured. It correctly uses the new ESLint flat config format and includes the recommended configurations from the JSONC plugin.
7-10: LGTM! Correct file targeting and parser configuration.The configuration correctly targets both
package.jsonandpackage_draft.jsonfiles using a concise pattern. The use ofparserJsoncis appropriate for parsing these JSON files, allowing for potential comments in the JSON.
1-28: Overall, excellent implementation of ESLint rules for sorting entries.This ESLint configuration successfully implements the sorting functionality requested in the PR objectives. It uses modern ESLint practices with the flat config format and targets the specific needs of your project. The rules are well-defined to enforce sorting in both
package.jsonandpackage_draft.jsonfiles.Great job on improving the project's code organization!
11-27: LGTM! Sorting rules are well-configured. Verify custom field names.The sorting rules are correctly configured to enforce alphabetical ordering of entries, which aligns with the PR objectives. The use of "error" severity ensures strict adherence to the sorting rules.
Please verify that
allowScopesandallowPackagesare the correct field names in yourpackage.jsonandpackage_draft.jsonfiles. If these are custom fields specific to your project, they look good. If not, you may need to adjust thepathPatternvalues.To confirm the existence and structure of these fields, you can run the following command:
This command will output the contents of these fields if they exist, allowing you to verify their structure and presence.
✅ Verification successful
To complete our verification, we need to check if package_draft.json exists and if it contains similar fields. Let's run another script to gather this information.
LGTM! Configuration is correct for package.json. package_draft.json not present.
The verification process has confirmed that the
allowScopesandallowPackagesfields are correctly configured inpackage.jsonwith the expected structure. TheallowScopesfield is an array of strings containing various scope names, and theallowPackagesfield is an object with package names as keys and version specifications as values.However,
package_draft.jsondoes not exist in the current directory. This absence doesn't necessarily indicate an issue, as it might be an optional file or not required for this specific project.The ESLint configuration is correctly set up to handle these fields in
package.json. Ifpackage_draft.jsonis indeed needed for your project, you may want to create it with similarallowScopesandallowPackagesfields. Otherwise, the current configuration is sufficient forpackage.json.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence and structure of allowScopes and allowPackages fields # Test: Check for allowScopes and allowPackages in package.json and package_draft.json jq '.allowScopes, .allowPackages' package*.jsonLength of output: 285940
Script:
#!/bin/bash # Check for package_draft.json and its contents if [ -f package_draft.json ]; then echo "package_draft.json exists. Contents of allowScopes and allowPackages:" jq '.allowScopes, .allowPackages' package_draft.json else echo "package_draft.json does not exist in the current directory." fiLength of output: 160
README.md (3)
10-11: LGTM: Clear guidance on using CLIThe added note effectively emphasizes the importance of using the CLI for adding white-list entries. This aligns well with the PR objective of introducing CLI support and helps prevent potential issues from manual
package.jsonmodifications.
37-43: LGTM: Clear instructions for adding scopesThe new section on adding specific scopes using the CLI is well-written and provides a clear example. This addition completes the set of instructions for using the CLI to manage the white-list, aligning with the PR objectives.
Line range hint
1-52: Overall: Excellent improvements to README clarity and alignment with PR objectivesThe changes to the README.md file successfully address the PR objectives by:
- Introducing clear instructions for using the new CLI functionality.
- Providing comprehensive examples for adding packages and scopes to the white-list.
- Improving the overall structure and readability of the document.
These updates will greatly enhance the user experience for developers working with the project, making it easier to manage entries in a consistent and organized manner.
scripts/generate.js (2)
134-136: LGTM! Script execution is correctly implemented.The main function is called at the end of the file, which is the standard practice for Node.js scripts.
1-136: Overall, great implementation with room for minor enhancements.The
scripts/generate.jsfile successfully implements a CLI for adding packages and scopes, meeting the PR objectives. The code is well-structured, includes appropriate input validation, and implements sorting as required. The use of debug logging and dynamic output paths are commendable practices.To further improve the script:
- Consider using a command-line argument parsing library for more robust handling of user inputs.
- Implement consistent error handling across all functions.
- Enhance the help message to include information about the debug mode.
These enhancements will make the script more robust, user-friendly, and maintainable.
# Conflicts: # package.json
|
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: npm/eslint@8.57.1 |
之前的方式添加看了头皮发麻，大家添加的都很随意，也没有按照字母进行排序。
Summary by CodeRabbit
New Features
README.mdwith clearer instructions and examples for managing packages via CLI.Bug Fixes
.gitignoreto excludepackage_draft.json, preventing unnecessary tracking.Refactor