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

#2102 PropertiesDefaultProvider at first try to load properties from … #2107

Merged
merged 4 commits into from
Sep 18, 2023

Conversation

rimuln
Copy link

@rimuln rimuln commented Sep 5, 2023

…classpath

Copy link
Owner

@remkop remkop left a comment

Choose a reason for hiding this comment

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

This looks good overall, many thanks!

Will you be able to add a unit test please?

One minor point is that the user object of the CommandSpec may be null in some circumstances (for applications that use the programmatic API to create a CommandSpec, instead of reflection from annotated application classes).
So the logic needs a minor change to accommodate this.

Lumír Návrat added 2 commits September 18, 2023 12:28
… from classpath

- check if commandSpec.userObject is null
… from classpath

- addded test for read default values from resource classpath
@rimuln
Copy link
Author

rimuln commented Sep 18, 2023

Will you be able to add a unit test please?

something tried if not enough not sure if I get quickly deeper insights in tests

@remkop remkop merged commit 6b047ae into remkop:main Sep 18, 2023
@rimuln rimuln deleted the issue-2102 branch September 18, 2023 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants