Skip to content

Fix config props defaults and descriptions #76

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

Merged
merged 1 commit into from
Dec 5, 2024

Conversation

onobc
Copy link
Contributor

@onobc onobc commented Dec 4, 2024

This commit simplifies the client properties by setting the default values directly on the properties and removes markdown from the property javadocs.
This improves end user experience by adding default values and descriptions to the generated config props docs.

This commit simplifies the client properties by setting the default values
directly on the properties and removes markdown from the property javadocs.
This improves end user experience by adding default values and descriptions
to the generated config props docs.

Signed-off-by: Chris Bono <chris.bono@gmail.com>
@onobc onobc requested a review from dsyer December 4, 2024 22:43
*/
private String address = "static://localhost:9090";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

All of the following changes are of the same pattern "put defaults directly on the fields and only use plain text for javadocs" in order to appease the Automatic Metadata Genarator.

The fields previously had null to indicate if the user had changed the value (i.e. support merges). However, we are not doing in merges and only copying defaults when a new unknown channel is encountered. For this, a simple "copy the default channel" suffices.

The tradeoff for the "merge of properties" is we get default values in the generated config props.

assertThat(newChannel).usingRecursiveComparison().isEqualTo(defaultChannel);
}

@Test
void withUserSpecifiedValuesAreRetained() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

While this was previously supported w/ copyDefaultsFrom we were not using it.

@dsyer dsyer merged commit 9dbe763 into spring-projects:main Dec 5, 2024
1 check passed
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 this pull request may close these issues.

None yet

2 participants