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

Remove test only MapperMergeContext#root method #109966

Merged

Conversation

javanna
Copy link
Member

@javanna javanna commented Jun 20, 2024

MapperMergeContext exposes a package private static root method that is only used in tests. That makes the production code slightly harder to follow because there are two possible code-paths. This commit simplifies that by moving the logic around providing a mapping reason to the caller tests, and removes the additional root method that is not needed in prod code.

MapperMergeContext exposes a package private static root method that is only used in tests.
That makes the production code slightly harder to follow because there are two possible code-paths. This commit
simplifies that by moving the logic around providing a mapping reason to the caller tests, and removes
the additional root method that is not needed in prod code.
@javanna javanna added :Search Foundations/Mapping Index mappings, including merging and defining field types >refactoring v8.15.0 labels Jun 20, 2024
@elasticsearchmachine elasticsearchmachine added the Team:Search Meta label for search team label Jun 20, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-search (Team:Search)

Copy link
Member

@piergm piergm left a comment

Choose a reason for hiding this comment

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

Nice simplification, LGTM!

@javanna javanna merged commit 8e44985 into elastic:main Jun 21, 2024
15 checks passed
@javanna javanna deleted the refactoring/remove_mapper_merge_context_root branch June 21, 2024 08:02
javanna added a commit to javanna/elasticsearch that referenced this pull request Jun 21, 2024
MapperMergeContext exposes a package private static root method that is only used in tests.
That makes the production code slightly harder to follow because there are two possible code-paths. This commit
simplifies that by moving the logic around providing a mapping reason to the caller tests, and removes
the additional root method that is not needed in prod code.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>refactoring :Search Foundations/Mapping Index mappings, including merging and defining field types Team:Search Meta label for search team v8.15.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants