itsaryan72 commented 1 month ago

User description

GENERAL: Only allow PNPM #366

description

Restriction on Package Manager Usage To ensure consistency in our project, I've added a preinstall script in package.json. "preinstall": "if [[ \"$npm_config_user_agent\" != pnpm ]]; then echo 'This project uses pnpm. Please run using pnpm.'; exit 1; fi",

This script checks the package manager before installation and displays an error if anything other than PNPM is used, promoting uniformity among contributors.

Fixes #366 GENERAL: Only allow PNPM #366

Dependencies

No additional packages or dependencies are required for the preinstall script

Developer's checklist

[Y] My PR follows the style guidelines of this project
[Y] I have performed a self-check on my work

If changes are made in the code:

[Y] I have followed the coding guidelines
[Y] My changes in code generate no new warnings
[ ] My changes are breaking another fix/feature of the project
[ ] I have added test cases to show that my feature works
[ ] I have added relevant screenshots in my PR
[Y] There are no UI/UX issues

Documentation Update

[ ] This PR requires an update to the documentation at docs.keyshade.xyz
[ ] I have made the necessary updates to the documentation, or no documentation changes are required.

PR Type

enhancement, configuration changes

Description

Added a preinstall script in package.json to enforce the use of PNPM as the package manager.
The script checks the npm_config_user_agent and exits with an error message if a package manager other than PNPM is used.
This change ensures consistency and uniformity among contributors by restricting package manager usage.

Changes walkthrough 📝

Relevant files

Configuration changes

package.json

Enforce PNPM as the package manager in preinstall script

package.json

Added a preinstall script to enforce the use of PNPM.

The script checks the package manager and exits with an error if not
PNPM.

+1/-0

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

codiumai-pr-agent-free[bot] commented 1 month ago

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Key issues to review

Compatibility
The preinstall script may not work correctly on Windows systems due to the use of Bash-specific syntax.

codiumai-pr-agent-free[bot] commented 1 month ago

PR Code Suggestions ✨

Category	Suggestion	Score
Enhancement	Improve the accuracy of the package manager check in the preinstall script ___ Consider using a more robust check for PNPM by explicitly checking for 'pnpm/' at the start of the user agent string. This will prevent false positives if 'pnpm' appears elsewhere in the user agent string. [package.json [93]](https://github.com/keyshade-xyz/keyshade/pull/467/files#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519R93-R93) ```diff -"preinstall": "if [[ \"$npm_config_user_agent\" != pnpm ]]; then echo 'This project uses pnpm. Please run using pnpm.'; exit 1; fi", +"preinstall": "if [[ \"$npm_config_user_agent\" != pnpm/* ]]; then echo 'This project uses pnpm. Please run using pnpm.'; exit 1; fi", ``` - [ ] Apply this suggestion Suggestion importance[1-10]: 8 Why: The suggestion improves the robustness of the check by ensuring that 'pnpm/' is at the start of the user agent string, reducing the chance of false positives.	8
User experience	Provide more helpful information in the error message of the preinstall script ___ Consider adding a more informative error message that includes instructions on how to install PNPM if it's not already installed. [package.json [93]](https://github.com/keyshade-xyz/keyshade/pull/467/files#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519R93-R93) ```diff -"preinstall": "if [[ \"$npm_config_user_agent\" != pnpm ]]; then echo 'This project uses pnpm. Please run using pnpm.'; exit 1; fi", +"preinstall": "if [[ \"$npm_config_user_agent\" != pnpm ]]; then echo 'This project uses pnpm. Please run using pnpm. To install pnpm, visit https://pnpm.io/installation'; exit 1; fi", ``` - [ ] Apply this suggestion Suggestion importance[1-10]: 7 Why: Adding instructions on how to install PNPM enhances user experience by providing immediate guidance, though it is not crucial for functionality.	7

💡 Need additional feedback ? start a PR chat

rajdip-b commented 1 month ago

@kriptonian1

kriptonian1 commented 1 month ago

Hey @itsaryan72, it is returning Unsupported Protocol error than returning the intended messages, can you look into this