Defaults for jre.path and headerType #339
Conversation
| getLog().warn("headerType not set, defaulting to \"console\""); | ||
| headerType = "console"; |
There was a problem hiding this comment.
Wouldn't be better to make the headerType attribute required with a default value like this?
@Parameter(required = true, defaultValue = "console")
private String headerType;There was a problem hiding this comment.
Sure, if you want a single default value with no 'magic' that's better
| if (classPath.mainClass.toLowerCase().contains("gui")) { | ||
| getLog().warn("headerType not set, defaulting to \"gui\" because the main class is named " + classPath.mainClass); | ||
| headerType = "gui"; | ||
| return; | ||
| } |
There was a problem hiding this comment.
I don't like such magic, it's can be confusing to users why it behaves like this.
| if (jre == null) { | ||
| jre = new Jre(); | ||
| } | ||
|
|
||
| if (jre.path == null) { | ||
| String pathDef = "%JAVA_HOME%;%PATH%"; | ||
| getLog().warn("jre.path not set, defaulting to \"" + pathDef + "\""); | ||
| jre.path = pathDef; | ||
| } |
There was a problem hiding this comment.
jre.path cannot be null if defined via pom.xml, the only case to have it null is when you create it by hand like this. I would enclose the whole logic with if:
if (jre == null) {
jre = new Jre();
String pathDef = "%JAVA_HOME%;%PATH%";
getLog().info("jre not defined, defaulting jre & jre.path to \"" + pathDef + "\"");
jre.path = pathDef;
}There was a problem hiding this comment.
Can we keep it just in case? Better some unnecessary checks now than surprises later
|
I can issue a new release but I think it should be a new |
|
Sure, I was thinking of suggesting that |
Path defaults to %JAVA_HOME%;%PATH% which is probably what most users want
Header type defaults to console, unless the main class has "gui" in full name, then to gui.
Closes #338