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: monitor support for --all-projects #929

Merged
merged 5 commits into from
Jan 5, 2020

Conversation

lili2311
Copy link
Contributor

@lili2311 lili2311 commented Dec 27, 2019

  • Ready for review
  • Follows CONTRIBUTING rules
  • Reviewed by Snyk internal team

What does this PR do?

  • when --all-project flag is enabled, auto detect => scan and monitor all supported projects (yarn, npm, rubygems, maven) in the given directory only initially
  • refactor to get deps the exact same way for test & monitor

Where should the reviewer start?

Relies on this refactor to go in first: #928

@lili2311 lili2311 requested a review from a team as a code owner December 27, 2019 16:37
@lili2311 lili2311 self-assigned this Dec 27, 2019
@ghost ghost requested review from dkontorovskyy and orsagie December 27, 2019 16:37
@lili2311 lili2311 changed the title Feat/monitor all projects Feat: monitor all projects Dec 27, 2019
@lili2311 lili2311 changed the title Feat: monitor all projects feat: monitor support for --all-projects Dec 27, 2019
@lili2311 lili2311 added the 🚧 WIP Work In Progress label Dec 27, 2019
@lili2311 lili2311 force-pushed the feat/monitor-all-projects branch 5 times, most recently from 55887fc to 0294534 Compare December 27, 2019 17:25
@@ -0,0 +1,25 @@
import { GoodResult, BadResult } from './types';

export function processJsonMonitorResponse(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

copied as is

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • types

@lili2311 lili2311 force-pushed the feat/monitor-all-projects branch 6 times, most recently from 5bfe017 to bc65d44 Compare December 30, 2019 12:07
// TODO: the type should depend on allSubProjects flag
const inspectResult: pluginApi.InspectResult = await promiseOrCleanup(
moduleInfo.inspect(path, targetFile, { ...options }),
const inspectResult = await promiseOrCleanup(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

key change

@dkontorovskyy dkontorovskyy force-pushed the feat/monitor-all-projects branch 2 times, most recently from c7afcd7 to 702038a Compare December 30, 2019 13:22
@lili2311 lili2311 force-pushed the feat/monitor-all-projects branch 4 times, most recently from d330103 to 5ade302 Compare December 30, 2019 18:07
}

throw new Error(json);
return processJsonMonitorResponse(results);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

separate PR?

@lili2311 lili2311 force-pushed the feat/monitor-all-projects branch 3 times, most recently from d018a1c to 0988aa3 Compare December 31, 2019 17:36
@dkontorovskyy dkontorovskyy force-pushed the feat/monitor-all-projects branch 4 times, most recently from 662cb78 to 573e59d Compare January 1, 2020 17:27
@lili2311 lili2311 removed the 🚧 WIP Work In Progress label Jan 2, 2020
@lili2311 lili2311 force-pushed the feat/monitor-all-projects branch 2 times, most recently from 8dc3a94 to 39deb39 Compare January 2, 2020 18:07
src/cli/index.ts Outdated
'project-name or file or package-manager or docker',
'all-projects',
]);
if (args.options['project-name'] && args.options.allProjects) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe this should be a separate function that validates arguments?

@lili2311 lili2311 force-pushed the feat/monitor-all-projects branch 2 times, most recently from 0b579a7 to a528b27 Compare January 3, 2020 15:19
package.json Outdated
@@ -28,7 +28,7 @@
"format": "prettier --write '{src,test,scripts}/**/*.{js,ts}'",
"prepare": "npm run build",
"test:common": "npm run check-tests && npm run build && npm run lint && node --require ts-node/register src/cli test --org=snyk",
"test:acceptance": "tap test/acceptance/**/*.test.* -Rspec --timeout=300 --node-arg=-r --node-arg=ts-node/register",
"test:acceptance": "tap test/acceptance/cli-test/*.test.* -Rspec --timeout=300 --node-arg=-r --node-arg=ts-node/register",
Copy link
Contributor

Choose a reason for hiding this comment

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

revert this?

@lili2311 lili2311 force-pushed the feat/monitor-all-projects branch 2 times, most recently from f224c20 to 04fe3c4 Compare January 3, 2020 17:06
@@ -9,7 +9,7 @@ export function formatTestMeta(
): string {
const padToLength = 19; // chars to align
const packageManager = res.packageManager || options.packageManager;
const targetFile = res.targetFile || options.file;
const targetFile = res.targetFile || res.displayTargetFile || options.file;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is used for display purposes and not being sent to the back end

@dkontorovskyy dkontorovskyy merged commit d6219f9 into master Jan 5, 2020
@dkontorovskyy dkontorovskyy deleted the feat/monitor-all-projects branch January 5, 2020 12:16
@snyksec
Copy link

snyksec commented Jan 5, 2020

🎉 This PR is included in version 1.274.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants