[WIP] CLOUDSTACK-10161: Use Maven wrapper#2343
Conversation
|
This will replace one tool dependency (mvn) to another (mvnw) and also addition of library (.jar file) in codebase is not recommended. |
|
IMO It's not just replacing one tool with another, it:
|
|
Can't we download that binary (jar file) in the script execution? |
|
@rafaelweingartner that defeats the whole point of using |
|
@khos2ow that is very interesting. They are indeed versioning this jar file. I always had Spring projects as an example of quality; I find versioning binary files a bit unusual though. However, if we zoom out the issue we see that we already version binary files when versioning pictures (jpg, png, and others) for instance. There are even ".class" files in our repo (https://github.com/apache/cloudstack/blob/master/tools/appliance/convert/Convert.class). I would be ok with this change. |
81e9e9d to
1a29ff3
Compare
|
@khos2ow can you rebase this PR to latest master commit, and then we can check Jenkins and Travis status again. Let's make sure both are green to proceed with reviews |
|
@rafaelweingartner I already rebased today after I saw your comment, but the thing is this PR is really in WIP and I haven't completed it yet. I'll try to do it Soon™! |
|
Ok, thanks! |
|
@rafaelweingartner actually I noticed that going forward with this PR depends on #2433, because they both touch building and packaging scripts (mostly). |
|
no problem, so we focus there first. |
|
@khos2ow are you planning to pick this up again? |
DaanHoogland
left a comment
There was a problem hiding this comment.
i am not convinced that we should package build tools, though I agree we should ensure build tool versions in our build scripts. Having people ensure their build environment doesn't seem like a stranger thing to demand of developers or devops engineers. We could even provide a install-tools script to facilitate this, ensuring correct versions.
i am -0 (as opposed to +0) on this
| # yum -y install mysql-server | ||
| # yum -y install git | ||
| # yum -y install genisoimage | ||
| $ yum -y update |
There was a problem hiding this comment.
this is not valid. the prompt you could run yum from on most syustems would be '#'
|
@rafaelweingartner yes, I still think this would make sense to go forward with, but since I created a couple of docker |
|
Can I close this then? |
|
Sure, closed it. |
Use maven wrapper to be used as
./mvnw foo bar -Pbazinstead ofmvn foo bar -Pbaz.