-
Notifications
You must be signed in to change notification settings - Fork 11
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: init support mvn -D verbose support #164
Conversation
8d27ac6
to
0809635
Compare
8ad43b0
to
bcbf2d4
Compare
c390e5f
to
e7accac
Compare
830c4c4
to
cae7438
Compare
181c0c2
to
4cf2286
Compare
9644170
to
5c1858a
Compare
5c1858a
to
289b5f0
Compare
lib/index.ts
Outdated
@@ -179,7 +179,10 @@ export async function inspect( | |||
cwd: mvnWorkingDirectory, | |||
}, | |||
); | |||
const parseResult = parse(result, options.dev); | |||
|
|||
const includesVerboseFlag = mvnArgs.includes('-Dverbose'); |
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.
not very likely, but what if the command is -Dverbose=false
, which is a user trying to disable verbose
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 is a good one, we will make sure of double check that -Dverbose will be false 👍
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 wasn't expecting us to reimplement so much. Can we not use -Dverbose
with -DoutputType=dot
?
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.
Not in all versions of the plugin, for this reason we have introduced this change, even though our preference is always -DoutputType=dot
return parseDependency(line); | ||
} | ||
|
||
private computeDepth(line: string): number { |
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.
is this reliable? i think it may break depending what terminal you're running - it's the reason we use dot output
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.
we have overly stressed this, as I said it's an incremental change for cases where mvn plugin does not support -Doutputype=dot with -DVerbose
return { | ||
plugin: { | ||
name: 'bundled:maven', | ||
runtime: 'unknown', | ||
meta: { | ||
versionBuildInfo: { | ||
metaBuildVersion: { | ||
mavenVersion, | ||
mavenFullVersion, |
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 mavenVersion: mavenFullVersion
Co-authored-by: Or Sagie or@snyk.io With this pull request if an user asks for -Dverbose arg that invalidates -DoutputType=dot We will still digest the mvn cli output despite no longer having a digraph DOT format but a mvn tree. We generate depGraph/s for single and/or aggregate projects.
9372b43
to
ee6264d
Compare
Co-authored-by: Or Sagie or@snyk.io
With this pull request if an user asks for -Dverbose arg that invalidates -DoutputType=dot
We will still digest the mvn cli output despite no longer having a digraph DOT format but a mvn tree.
We generate depGraph/s for single and/or aggregate projects.