-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Rework camel-jbang-it to support camel-launcher #21362
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
base: main
Are you sure you want to change the base?
Conversation
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🤖 CI automation will test this PR automatically. 🐫 Apache Camel Committers, please review the following items:
|
ad00ff2 to
d476966
Compare
|
|
||
| protected String executeBackground(final String command) { | ||
| return containerService.executeBackground(command); | ||
| return containerService.executeGenericCommand(MAIN_COMMAND + " " + command + " --background"); |
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.
why you are implementing again what we already have in containerService.executeBackground() ? is this a different behaviour?
|
|
||
| private static final String DATA_FOLDER = System.getProperty(CliProperties.DATA_FOLDER); | ||
|
|
||
| private static final String MAIN_COMMAND = System.getProperty("cli.service.command"); |
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.
what do you think to implement a getMainCommand() method in the CliService class so that we have just one place to retrieve the property? you can use so containerService.getMainCommand() instead of MAIN_COMMAND in all the places
These changes allow customization of the
camelcommand in order to supportcamel-launcher.There are also differences from the closed PR with the same title. This one only contains the rework itself alongside some minor fixes. I also greatly simplified it by removing the property out of the container service and keeping it as a simple system property as @mcarlett suggested.