-
Notifications
You must be signed in to change notification settings - Fork 84
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 Maven Wrapper support #152
Conversation
@rhuss & Co. how about this? I could use this to throw https://github.com/vorburger/opendaylight-bot on OpenShift... PS: This will probably conflict with some of my other pending PRs; get those in first, then I'll rebase it. |
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 in general, but I wonder whether we should include Maven wrapper support also in the RHEL images.
I.e. to pick up a local mvn when provided in the source code to compile.
wdyt, @jamesnetherton @dhirajsb ?
I think we should only support this in the community images. |
@jamesnetherton so how exactly do we want to "condition" this? Similar to the Gradle "feature switch" in #121, so if I added a |
@vorburger yes, a simple feature switch would be fine. |
java/images/jboss/s2i/assemble
Outdated
@@ -137,14 +137,16 @@ function build_maven() { | |||
|
|||
local mvn="mvn" | |||
if [ -f "mvnw" ]; then | |||
echo "Using local mvnw" | |||
./mvnw --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 is kind of redundant with line 148 below, but never mind
@jamesnetherton @rhuss done; |
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.
thanks, lgtm
see https://github.com/takari/maven-wrapper