Issue #5256 Adding start.jar --add-module #5258
Conversation
Signed-off-by: Greg Wilkins <gregw@webtide.com>
usage.txt Signed-off-by: Greg Wilkins <gregw@webtide.com>
joakime
changed the title
|
@gregw can you please add a check to abort start if there is an unrecognized option? For example: This still starts the server, I think it's better to abort with "Unrecognized option |
| @@ -128,7 +128,7 @@ protected Path getDestination(URI uri, String location) throws IOException | |||
| protected void download(URI uri, Path destination) throws IOException | |||
| { | |||
| if (FS.ensureDirectoryExists(destination.getParent())) | |||
| StartLog.log("MKDIR", _basehome.toShortForm(destination.getParent())); | |||
| StartLog.log("INFO", "mkdir " + _basehome.toShortForm(destination.getParent())); | |||
There was a problem hiding this comment.
I kind of liked the previous format, although it was not consistent (e.g. was using INFO for module activity).
If you go this direction, then there are 3 occurrences of StartLog.log("COPY", ...) and 1 of StartLog.log("DOWNLD", ...) to replace.
There was a problem hiding this comment.
I'm torn.... but on balance I think that INFO/WARN are least surprise.
|
@gregw also:
should be |
updates from review Signed-off-by: Greg Wilkins <gregw@webtide.com>
|
@sbordet If we want to warn about unknown args, we will have to get rid of |
I retract, we want unknown arguments to |
| { | ||
| StartLog.log("MKDIR", baseHome.toShortForm(startd)); | ||
| StartLog.info("create " + baseHome.toShortForm(startini)); |
There was a problem hiding this comment.
This should be mkdir for consistency with other logs, not create.
There was a problem hiding this comment.
but it is not a directory! "touch" ?
There was a problem hiding this comment.
Changed from startd to startini ?
There was a problem hiding this comment.
@joakime exectly because the option is now --create-start-ini and --create-startd is a noop
There was a problem hiding this comment.
Note that --create-start-ini takes all the files in start.d and concats them into a start.ini and then removes start.d
jetty-start/src/main/java/org/eclipse/jetty/start/Licensing.java
Outdated
Show resolved
Hide resolved
jetty-start/src/main/java/org/eclipse/jetty/start/builders/StartDirBuilder.java
Outdated
Show resolved
Hide resolved
updates from review Signed-off-by: Greg Wilkins <gregw@webtide.com>
Signed-off-by: Greg Wilkins <gregw@webtide.com>
Fixes #5256