-
Notifications
You must be signed in to change notification settings - Fork 582
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
fix: reinstate --all-sub-project support for Gradle #732
Conversation
27ecb30
to
04d2be3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If changing to scannedProject, please do so everywhere.
src/lib/snyk-test/run-test.ts
Outdated
}; | ||
} else { | ||
// We are using "options" to store some information returned from plugin that we need to use later, | ||
// but don't want to send to Registry in the Payload. | ||
// TODO(kyegupov): decouple inspect and payload so that we don't need this hack | ||
options.subProjectNames = inspectRes.depRoots.map((depRoot) => depRoot.depTree.name); | ||
options.subProjectNames = inspectRes.scannedProjects.map((depRoot) => depRoot.depTree.name); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change to scannedProject variable name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed (+ all mentions of "depRoot" ever)
src/lib/snyk-test/run-test.ts
Outdated
@@ -263,7 +262,7 @@ async function assembleLocalPayloads(root, options: Options & TestOptions): Prom | |||
const deps = await getDepsFromPlugin(root, options); | |||
analytics.add('pluginName', deps.plugin.name); | |||
|
|||
for (const depRoot of deps.depRoots) { | |||
for (const depRoot of deps.scannedProjects) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
@@ -632,7 +632,7 @@ test('`test gradle-app --all-sub-projects` returns correct multi tree meta', asy | |||
async inspect() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add the return type here, so we know it's the right one we are building?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added
04d2be3
to
2a0b445
Compare
2a0b445
to
2c5533a
Compare
2c5533a
to
047998e
Compare
🎉 This PR is included in version 1.216.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
What does this PR do?
Fixes
--all-sub-project
flag broken by the Gradle plugin upgrade.Also, fully use the plugin type definitions from the snyk-cli-interface package.