Skip to content
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

feat: support devDependencies on package.json parser #174

Merged

Conversation

arthurdenner
Copy link
Contributor

@arthurdenner arthurdenner commented Mar 15, 2022

Fixes #173.

Screenshot

Screenshot 2022-03-15 at 15 33 46

As --dev is the trigger for scanning devDependencies, I've approached the issue like this:

  1. Pass the additional CLI parameters to the package.json parser;
  2. Look for the --dev string inside the parameters (if provided)
  3. Check if there are devDependencies in the package.json
  4. If 2 and 3 are true, parse the devDependencies as the dependencies

Let me know if you have suggestions for changes or if I have approached the issue incorrectly.

@arthurdenner arthurdenner requested a review from a team as a code owner March 15, 2022 14:39
@CLAassistant
Copy link

CLAassistant commented Mar 15, 2022

CLA assistant check
All committers have signed the CLA.

@michelkaporin
Copy link
Contributor

Hi @arthurdenner! Thank you for your PR. Since we don't often receive product functionality contributions we need to run it by team and I'll get back to you right afterwards. The code looks good so far ✔️ Apologise for the delay.

@michelkaporin
Copy link
Contributor

@arthurdenner Apologise for taking it too long. We iterated with the team on how should we approach feature contributions. For instance, given we have other IDE plugins, we're trying to ensure we're consistent with our product functionality across all of them. That means if feature lands in VS Code extension, it should be also added to IntelliJ plugin, Visual Studio extension, etc.

All in all we're ready to accept your PR, please resolve the conflicts and let me know. Thank you!

@arthurdenner arthurdenner force-pushed the feat/support-devDeps-package.json branch 2 times, most recently from 3be3834 to 97a9dc5 Compare April 8, 2022 14:44
@arthurdenner
Copy link
Contributor Author

@michelkaporin, I've updated the PR after solving the conflicts.
Let me know if there's any change to do! ✌🏽

@arthurdenner arthurdenner force-pushed the feat/support-devDeps-package.json branch from 97a9dc5 to ad0d2e9 Compare April 8, 2022 14:48
michelkaporin
michelkaporin previously approved these changes Apr 8, 2022
Copy link
Contributor

@michelkaporin michelkaporin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! Thank you for your contribution 👏

Update: Just realised your commits should be signed before we merge in, please sign them 🙏

@arthurdenner
Copy link
Contributor Author

@michelkaporin, done! Apologies for the delay.

@michelkaporin michelkaporin merged commit 86d0d45 into snyk:main Apr 19, 2022
@arthurdenner arthurdenner deleted the feat/support-devDeps-package.json branch April 19, 2022 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Vulnerabilities in devDependencies don't show on package.json
3 participants