-
Notifications
You must be signed in to change notification settings - Fork 567
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: convert nodejs plugin response to use multi format #773
Conversation
1783fb2
to
01aef00
Compare
src/cli/commands/monitor.ts
Outdated
@@ -190,18 +190,18 @@ async function monitor(...args0: MethodArgs): Promise<any> { | |||
const manageUrl = url.format(endpoint); | |||
|
|||
endpoint.pathname = leader + '/monitor/' + res.id; | |||
const subProjectName = pluginApi.isMultiResult(inspectResult) | |||
const projectName = (pluginApi.isMultiResult(inspectResult)) | |||
? subProjDeps.package.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.
Let's consistently rename subProj
as well.
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.
Updated @miiila
const depTree: any = getLockFileDeps ? | ||
await lockParser.parse(root, targetFile, options) : | ||
await modulesParser.parse(root, targetFile, options); | ||
|
||
return { | ||
plugin: { | ||
name: 'snyk-nodejs-lockfile-parser', | ||
runtime: process.version, |
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.
This may be contradicting this change: https://github.com/snyk/snyk-cli-interface/pull/21/files#diff-b2d5ac8d1346936f02713d534de45340R93
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.
Don't see an issue there
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.
the issue will be that different projects will be scanned soon and they will not share always the exact same version of gradle or maven or sbt for example. Or node in this example.
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.
But we can fix that easily I think but adding meta inside each result not on a top level response
01aef00
to
1e8eec8
Compare
1e8eec8
to
9dbfb79
Compare
9dbfb79
to
fb43699
Compare
Manually tested, works as expected 🎉 |
@@ -28,7 +28,7 @@ interface GoodResult { | |||
ok: true; | |||
data: string; | |||
path: string; | |||
subProjectName?: string; | |||
projectName?: string; |
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.
I like this change of renaming this from subProjectName
to projectName
. We don't really have a concept of sub project, as we monitor each sub project as separate project.
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.
LGTM
🎉 This PR is included in version 1.232.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
What does this PR do?
Convert the nodejs plugin code to use the new multi result format in preparation for auto detections and handling of multiple file results.