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.
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
[Enhancement] Add schema validation and placeholders to index mappings #3240
base: main
Are you sure you want to change the base?
[Enhancement] Add schema validation and placeholders to index mappings #3240
Changes from 30 commits
18eedf6
7068729
abd4b25
d2f3c0e
fa81560
63a6d62
8cbaf01
3423026
67a1810
63bc70d
88673ed
409fbd9
9776787
af4d1d0
b383b48
56d945b
4ea99ea
6bef762
ba9d232
ce72e53
e714c35
d50a9f9
86d2b76
1cd16b2
369475d
313b70d
d8390ca
cba2c88
d7b19ea
2884b50
3e45f6e
399b892
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to have this check here considering we are going to check
mapping.isBlank()
in thereplacePlaceholders
method. Seems like redundant.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here we are creating string in every replace. May be we could use a StringBuilder to have in-place replacement?
May be something like this:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor: this seems not a standard java doc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
made it into a proper comment for now via latest commit, i will modify to java doc standard with
*
in a later pr, i don't want to dismiss the previous approval with a new push again, if thats okayThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add try catch block to catch any exceptions like:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about this: