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

Inconsistent naming and location for index level settings #19723

Closed
abeyad opened this issue Aug 1, 2016 · 1 comment
Closed

Inconsistent naming and location for index level settings #19723

abeyad opened this issue Aug 1, 2016 · 1 comment
Labels
:Core/Infra/Settings Settings infrastructure and APIs discuss

Comments

@abeyad
Copy link

abeyad commented Aug 1, 2016

Currently, we find the index level settings all over the place, both in terms of the location (class) in which they are declared and naming conventions.

In terms of naming, a major source of inconsistency is whether to name the setting SETTING_{name} or {name}_SETTING (e.g. SETTING_WAIT_FOR_ACTIVE_SHARDS or WAIT_FOR_ACTIVE_SHARDS_SETTING). It would be good to settle on a convention for readability and consistency, and so we don't have to resort to discussions on every PR about what the proper name should be.

In terms of location, we have many index level settings in IndexSettings, many in IndexMetaData, then others scattered around in other places specific to the class that it is most associated with (e.g. index.mapping.coerce which is found in FieldMapper#COERCE_SETTING). It would be good to discuss if we are fine with this ad hoc approach to locating index settings, or if we want some more consistency, where should they all go? Should there be a dedicated constants class for this?

@rjernst
Copy link
Member

rjernst commented Mar 14, 2018

Regarding naming, I have always used {name}_SETTING, and while I do see quite a few settings begging with SETTING_, I think these are the minority, and I have not seen recent settings being added with this convention.

Regarding location, I think this is similar to #19878. For the same reasons there, I am going to close this. Having an issue has not been a driving factor in increasing the priority of such a refactoring.

@rjernst rjernst closed this as completed Mar 14, 2018
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 discuss
Projects
None yet
Development

No branches or pull requests

2 participants