Refactor enum mappings parameter to allow for capital case types #92548
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We have a couple of existing enum mappings parameters that specify they enum type in lowercase letters. That is convenient to avoid having to convert enum names back and to uppercase or lowercase depending on what's needed, yet it does not follow coding conventions in that constants should be in capital letters. More importantly, moving the `on_script_error` mapping parameter to a streamlined enum mapping parameter is not possible with lowercase type names because one of its values is `continue` which is a java reserved keyword. It becomes a requirement that the actual value for an enum based mapping parameter can potentially differ from the enum name. In general, the type name will be in capital letters, while the parameter value will be lowercase. With this commit we refactor the enum mappings parameter to provide their types in capital case, while users will keep on providing the corresponding values in lowercase. This only affects how the enum types are represented internally. We can leverage toString for the enum types to do the lowercasing when needed.
javanna
added
:Search Foundations/Mapping
|
Pinging @elastic/es-search (Team:Search) |
danielmitterdorfer
pushed a commit
to danielmitterdorfer/elasticsearch
that referenced
this pull request
Jan 12, 2023
…stic#92548) We have a couple of existing enum mappings parameters that specify they enum type in lowercase letters. That is convenient to avoid having to convert enum names back and to uppercase or lowercase depending on what's needed, yet it does not follow coding conventions in that constants should be in capital letters. More importantly, moving the `on_script_error` mapping parameter to a streamlined enum mapping parameter is not possible with lowercase type names because one of its values is `continue` which is a java reserved keyword. It becomes a requirement that the actual value for an enum based mapping parameter can potentially differ from the enum name. In general, the type name will be in capital letters, while the parameter value will be lowercase. With this commit we refactor the enum mappings parameter to provide their types in capital case, while users will keep on providing the corresponding values in lowercase. This only affects how the enum types are represented internally. We can leverage toString for the enum types to do the lowercasing when needed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We have a couple of existing enum mappings parameter that specify they enum type in lowercase letters. That is convenient to avoid having to convert back and forth lowercase and uppercase enum names, yet it does not follow coding conventions that constants should be in capital letters. More importantly, moving the
on_script_errormapping parameter to a streamlined enum mapping parameter is not possible with lowercase type names because one of its values iscontinuewhich is a java reserved keyword.With this commit we refactor the enum mappings parameter to provide their types in capital case, while users will keep on providing the corresponding values in lowercase. This only affects how the enum types are represented internally.