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
Fix package maven #1320
base: master
Are you sure you want to change the base?
Fix package maven #1320
Conversation
✅ Deploy Preview for spaceship-prompt ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
d83dab9
to
c14d732
Compare
local maven_exe=$(spaceship::upsearch mvnw) || (spaceship::exists mvn && maven_exe="mvn") || return | ||
|
||
$maven_exe help:evaluate -q -DforceStdout -D"expression=project.version" 2>/dev/null | ||
local maven_exe=$(spaceship::upsearch mvnw || (spaceship::exists mvn && echo "mvn")) || return |
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 very complex line. Can we simplify that with a multi-line if-else
?
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.
Sorry @denysdovhan I totally missed this. Sure, I can clean things up. 👍
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 a problem. Waiting for your changes.
Description
The package maven module didn't handle non-parseable XML file (pom.xml). This could happen e.g. if you are in a middle of a merge-conflict. The maven command prints errors to stdout so we have to check the exit code.
When fixing non-parseable pom I discovered that we didn't handle the lookup for maven exe correctly so I fixed that too.
Added tests for package_maven.
Screenshot