-
Notifications
You must be signed in to change notification settings - Fork 60
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
Add simple Maven Plugin #49
Conversation
This relates to issue #48. |
org.eclipse.transformer.maven/src/main/java/org/eclipse/transformer/maven/TransformMojo.java
Show resolved
Hide resolved
this.project = project; | ||
} | ||
|
||
public MavenSession getSession() { |
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.
There is a lot of boiler plate getters and setters which are not really used in the mojo. Some the them seem to be for testing, but we can get away with default visibility (non-public) for them.
Also, no one seems to even use session.
<dependency> | ||
<groupId>javax.inject</groupId> | ||
<artifactId>javax.inject</artifactId> | ||
<version>1</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.
I think we should put all the dependency version management into the parent pom's pluginManagement section so we have everything nicely centralized.
<groupId>org.jboss.shrinkwrap</groupId> | ||
<artifactId>shrinkwrap-api</artifactId> | ||
<version>1.2.6</version> | ||
<scope>compile</scope> |
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 think this is just a test dependency, right?
<plugin> | ||
<groupId>org.apache.maven.plugins</groupId> | ||
<artifactId>maven-jar-plugin</artifactId> | ||
<version>3.2.0</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.
We already set the version in the parent pom's pluginManagement.
</plugin> | ||
<plugin> | ||
<groupId>org.apache.maven.plugins</groupId> | ||
<artifactId>maven-release-plugin</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 don't think we need this plugin.
I'll recheck in the morning, but I think I have managed to address all the feedback. Thanks for the review! |
Signed-off-by: Jonathan Gallimore <[email protected]>
It looks like the feedback here is addressed. I've rebased and resolved the conflicts. Would it be possible for someone to have another look? |
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
We discussed this a bit on the old repository, so here it is, rebased against the new Maven build. See tbitonti/jakartaee-prototype#102.
The Mojo provides a classifier parameter, and any attached artifacts get transformed, and the transformed artifacts are attached with whatever value is specified appended to the artifacts classifier.
For example, in the TomEE build we have the following plugin config:
And the module has the following attached artifacts:
org.apache.tomee:apache-tomee:webprofile:8.0.3-SNAPSHOT:zip
org.apache.tomee:apache-tomee:microprofile:8.0.3-SNAPSHOT:zip
org.apache.tomee:apache-tomee:plume:8.0.3-SNAPSHOT:zip
org.apache.tomee:apache-tomee:plus:8.0.3-SNAPSHOT:zip
Once the plugin is run, the following additional artifacts are attached:
org.apache.tomee:apache-tomee:webprofile-jakartaee9:8.0.3-SNAPSHOT:zip
org.apache.tomee:apache-tomee:microprofile-jakartaee9:8.0.3-SNAPSHOT:zip
org.apache.tomee:apache-tomee:plume-jakartaee9:8.0.3-SNAPSHOT:zip
org.apache.tomee:apache-tomee:plus-jakartaee9:8.0.3-SNAPSHOT:zip