Skip to content

Conversation

nicktindall
Copy link
Contributor

Broken out of #126091

@nicktindall nicktindall added >enhancement :Core/Infra/Settings Settings infrastructure and APIs labels Apr 14, 2025
@nicktindall nicktindall requested a review from a team as a code owner April 14, 2025 04:30
@elasticsearchmachine elasticsearchmachine added v9.1.0 Team:Core/Infra Meta label for core/infra team labels Apr 14, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@elasticsearchmachine
Copy link
Collaborator

Hi @nicktindall, I've created a changelog YAML for you.

Copy link
Contributor

@ldematte ldematte left a comment

Choose a reason for hiding this comment

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

LGTM

);
assertThat(
floatSetting.get(
Settings.builder().put("float_val.default", configuredDefaultValue).put("float_val", configuredSettingValue).build()
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: this would have been maybe more useful if it was something else, e.g.

assertThat( floatSetting.get( Settings.builder().put("float_val.default", someOtherDefaultValue).put("float_val", configuredSettingValue).build()), equalTo(configuredSettingValue) ); 
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it is, but my naming is not great.

i.e. defaultSetting (the default for float_val.default) != configuredDefault (the configured value for float_val.default != configuredSetting (the configured value for float_val). I'll fix the naming up to make that clearer.

@nicktindall nicktindall merged commit dfaf3de into elastic:main Apr 15, 2025
17 checks passed
@nicktindall nicktindall deleted the allow_float_settings_other_default branch April 15, 2025 06:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Core/Infra/Settings Settings infrastructure and APIs >enhancement Team:Core/Infra Meta label for core/infra team v9.1.0

3 participants