Conversation
luisrodero-fidesmo
left a comment
There was a problem hiding this comment.
LGTM! But I added a small recommendation about updating README to tell users that setting JAVA_HOME is mandatory (well, if that's the case).
| <mainClass>com.fidesmo.fdsm.Main</mainClass> | ||
| </classPath> | ||
| <jre> | ||
| <path>${env.JAVA_HOME}</path> |
There was a problem hiding this comment.
How bad is it if the user has not defined a JAVA_HOME? I ask because I'm not certain that all users will have that env var set. Maybe we can add something in the README explaining that JAVA_HOME is mandatory?
There was a problem hiding this comment.
Maven doesn't work without JAVA_HOME set at all. Also this specific goal is only run during the release phase which is a GH Action only and not intended for a local run, so don't think it needs additional documentation
There was a problem hiding this comment.
Ah ok, then it's good to merge.
This PR attempts to fix launch4j configuration, as starting from launch4j 3.50 JRE path is required. The 3.50 version was published after the last successfull release and even though we haven't updated the launch4j version ourselves, it seems the plugin takes the latest one automatically.
Locally the change works, so hope GH Action will like it as well.