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: detect other files & suggest to use --all-projects #1233

Merged
merged 1 commit into from Jul 1, 2020

Conversation

lili2311
Copy link
Contributor

@lili2311 lili2311 commented Jun 24, 2020

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

What does this PR do?

Suggest scanning with --all-projects when we detect other supported manifests up to 4 sub-directories deep. For both test and monitor to help discover the new feature.

Screenshots

Screen Shot 2020-06-24 at 22 33 55

Screen Shot 2020-06-24 at 22 34 06

Updated
Screen Shot 2020-07-01 at 13 33 36

@lili2311 lili2311 added the 🚧 WIP Work In Progress label Jun 24, 2020
@lili2311 lili2311 requested review from a team as code owners June 24, 2020 13:15
@lili2311 lili2311 self-assigned this Jun 24, 2020
@ghost ghost requested review from anthogez and MegaBean June 24, 2020 13:15
@lili2311 lili2311 added ☠️ tech services and removed 🚧 WIP Work In Progress labels Jun 24, 2020
(advertiseAllProjectsCount && foundProjectCount
? chalk.bold.white(
`Tip: Detected multiple supported manifests (${foundProjectCount}), ` +
'use --all-projects --detection-depth=4 to scan all of them at once.\n\n',
Copy link
Contributor

Choose a reason for hiding this comment

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

--detection-depth=4 is this now the default?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

our default is 2 which basically finds not much, 4 seemed to find much more, also it looks more impactful rather than suggesting to scan for the sake of 1 more file. What are thoughts on looking deeper by default?

Copy link
Contributor

Choose a reason for hiding this comment

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

yea think whatever we suggest (and use) here should be the default, if that's 4, lets make it default 4 too

try {
const extraTargetFiles = await find(root, [], AUTO_DETECTABLE_FILES, 4);
const foundProjectsCount =
extraTargetFiles.length > 1 ? extraTargetFiles.length - 1 : undefined;
Copy link
Contributor

Choose a reason for hiding this comment

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

is it more interesting to use which files (their path and name) we found? rather than just a count?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interesting, how would you stop this list from getting too big? file-a, file-b and 24 more... or something?

Copy link
Contributor

Choose a reason for hiding this comment

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

yea maybe truncate to a reasonable limit

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so changing both the message for gradle and this one? They both use the same ish wording & show how many more you could scan with an different option? This function handles both discovered gradle sub-proejcts (where we have the sub-project names) and now also extra manifests that you could scan with --all-projects

@github-actions
Copy link
Contributor

github-actions bot commented Jul 1, 2020

Expected release notes (by @lili2311)

features:
detect other files & suggest to use --all-projects (20b1811)

  • I hereby acknowledge these release notes are 🥙 AWESOME 🥙

@lili2311 lili2311 merged commit 94ce3e4 into master Jul 1, 2020
@lili2311 lili2311 deleted the feat/suggest-all-projects branch July 1, 2020 11:20
@snyksec
Copy link

snyksec commented Jul 1, 2020

🎉 This PR is included in version 1.358.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
3 participants