Skip to content
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

Cleanup Jetty 10 Start #5256

Closed
joakime opened this issue Sep 11, 2020 · 4 comments · Fixed by #5258
Closed

Cleanup Jetty 10 Start #5256

joakime opened this issue Sep 11, 2020 · 4 comments · Fixed by #5258
Assignees

Comments

@joakime
Copy link
Contributor

joakime commented Sep 11, 2020

Jetty version
10.0.0

Description
Some changes discussed.

  1. Change --list-modules shows a single line for each module. (the name and the first line of description)
  2. Add --show-modules for the full / detailed module list
  3. Deprecate --create-startd (show warning)
  4. The behavior of --create-startd is set to default mode, creating files in ${jetty.base}/start.d/ by default.
  5. Create new --create-start-ini to force older operation in start.ini
  6. Add new --add-modules=<name>,<name> option (plural form)
  7. The start.d vs start.ini default mode is dependent on the existence of start.ini (if it exists, the --add-modules=<name> default to adding to start.ini, otherwise it uses the new default of start.d)
@sbordet
Copy link
Contributor

sbordet commented Sep 11, 2020

@gregw we have add-modules already in the [jpms] directive in a *.mod file.
How about --enable-modules instead?

@gregw
Copy link
Contributor

gregw commented Sep 11, 2020

I think we need the word --add in there some where to distinguish between --module=foo which enables module foo for this run vs --add=foo which persistently adds the module to the base for all future runs.

If there is already a --add-modules, then I think we go with just -add as an abbreviation of either --add-to-start or --add-to-startd, whichever is in-use or the default.

@sbordet
Copy link
Contributor

sbordet commented Sep 11, 2020

@gregw however, both in the documentation and in the output of the command:

INFO  : server          transitively enabled, ini template available with --add-to-start=server
INFO  : logging-jetty   transitively enabled
INFO  : http            initialized in ${jetty.base}/start.d/http.ini
INFO  : resources       transitively enabled
INFO  : threadpool      transitively enabled, ini template available with --add-to-start=threadpool
INFO  : logging/slf4j   dynamic dependency of logging-jetty
INFO  : bytebufferpool  transitively enabled, ini template available with --add-to-start=bytebufferpool
MKDIR : ${jetty.base}/resources
COPY  : ${jetty.home}/modules/logging/jetty/resources/jetty-logging.properties to ${jetty.base}/resources/jetty-logging.properties
INFO  : Base directory was modified

The word "enabled" is mentioned multiple times - it's exactly what we do, we enable the module permanently.

@gregw
Copy link
Contributor

gregw commented Sep 11, 2020

  1. and 2. are done in Issue #5254 List/Show modules #5257

gregw added a commit that referenced this issue Sep 12, 2020
Signed-off-by: Greg Wilkins <[email protected]>
gregw added a commit that referenced this issue Sep 12, 2020
usage.txt

Signed-off-by: Greg Wilkins <[email protected]>
gregw added a commit that referenced this issue Sep 14, 2020
updates from review

Signed-off-by: Greg Wilkins <[email protected]>
gregw added a commit that referenced this issue Sep 14, 2020
updates from review

Signed-off-by: Greg Wilkins <[email protected]>
gregw added a commit that referenced this issue Sep 15, 2020
* Issue #5256 --add-modules

Signed-off-by: Greg Wilkins <[email protected]>

* main.ini

Signed-off-by: Greg Wilkins <[email protected]>

* Issue #5256 --add-modules

usage.txt

Signed-off-by: Greg Wilkins <[email protected]>

* Issue #5256 --add-modules

updates from review

Signed-off-by: Greg Wilkins <[email protected]>

* Issue #5256 --add-modules

updates from review

Signed-off-by: Greg Wilkins <[email protected]>

* fixed test

Signed-off-by: Greg Wilkins <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants