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

CAMEL-19313: resolve option placeholder #9987

Merged
merged 2 commits into from May 15, 2023
Merged

Conversation

Croway
Copy link
Contributor

@Croway Croway commented May 3, 2023

Description

Target

  • I checked that the commit is targeting the correct branch (note that Camel 3 uses camel-3.x, whereas Camel 4 uses the main branch)

Tracking

  • If this is a large change, bug fix, or code improvement, I checked there is a JIRA issue filed for the change (usually before you start working on it).

Apache Camel coding standards and style

  • I checked that each commit in the pull request has a meaningful subject line and body.
  • I formatted the code using mvn -Pformat,fastinstall install && mvn -Psourcecheck

@Croway Croway marked this pull request as draft May 3, 2023 12:04
@github-actions
Copy link
Contributor

github-actions bot commented May 3, 2023

🚫 There are (likely) no components to be tested in this PR

return doCall();
}

private void replacePlaceholders() throws Exception {
for (CommandLine.Model.ArgSpec argSpec : spec.args()) {
String defaultValue = spec.defaultValueProvider().defaultValue(argSpec);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the default values are the values set in configuration? (like when set with jbang camel@apache/camel config set repos=https://maven.repository.redhat.com/ga)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exactly, the defaultValueProvider() loads the value from the properties file (updated via camel config set)

@davsclaus
Copy link
Contributor

Can you maybe also add a little part in the camel-jbang doc about this

Copy link
Contributor

@apupier apupier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@github-actions github-actions bot added the docs label May 11, 2023
@Croway Croway marked this pull request as ready for review May 11, 2023 14:44
@Croway
Copy link
Contributor Author

Croway commented May 11, 2023

Thanks @apupier for checking it, I have added some doc too.

@github-actions
Copy link
Contributor

🚫 There are (likely) no components to be tested in this PR

@davsclaus
Copy link
Contributor

This is a draft PR is it ready to be merged or do you need more work?

@Croway Croway merged commit 723df5c into apache:main May 15, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants