-
Notifications
You must be signed in to change notification settings - Fork 28
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
Dependency graph #152
Dependency graph #152
Conversation
<modelVersion>4.0.0</modelVersion> | ||
|
||
<groupId>org.spdx</groupId> | ||
<artifactId>spdx-maven-plugin-test</artifactId> |
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 had to replace the whitespaces for this test case because it threw the following error when building the MavenProject using ProjectBuilder
:
org.apache.maven.project.ProjectBuildingException: Some problems were encountered while processing the POMs:
[ERROR] 'artifactId' with value 'spdx maven plugin test' does not match a valid id pattern. @ line 7, column 17
import org.spdx.storage.ISerializableModelStore; | ||
import org.spdx.storage.simple.InMemSpdxStore; | ||
|
||
public class TestWithSessionSpdxMojo extends AbstractMojoTestCase |
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 mojos in TestSpdxMojo
are created without a Maven session because they are loaded with lookupMojo
. I had to use lookupConfiguredMojo
to include a session to the mojo (and customize the session a bit). The session is used to generate the dependency graph.
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.
One change to retain the test dependencies and a small nit. Otherwise, looks good.
src/main/java/org/spdx/maven/utils/SpdxDependencyInformation.java
Outdated
Show resolved
Hide resolved
This fixes #87 |
Great! Thanks! Feedback applied. |
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. Thanks @rogelio-o
Type of change
Changes proposed in this pull request
Project name: Dependency graph #151
Description