-
Notifications
You must be signed in to change notification settings - Fork 24.9k
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
Use collections conveniences in static initializers #41374
Conversation
This commit replaces the construction of some collections in static initializers with new collection convenience methods that are available now that we have bumped the minimum Java language level to be higher than Java 8.
Pinging @elastic/es-core-infra |
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.
Nice, it makes things easier to read.
@@ -128,13 +123,13 @@ private QueryAnalyzer() { | |||
* @param indexVersion The create version of the index containing the percolator queries. | |||
*/ | |||
static Result analyze(Query query, Version indexVersion) { | |||
Class<?> queryClass = query.getClass(); | |||
Class<? extends Query> queryClass = query.getClass(); |
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.
I think this is creating more problems than it helps?
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.
@@ -99,7 +99,11 @@ private Sets() { | |||
public static <T> SortedSet<T> sortedDifference(Set<T> left, Set<T> right) { |
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.
Not directly related to you change, but I'm seeing some inconsistencies wrt whether the return value of this method is assumed mutable. Reconfigurator
seems to assume it is immutable and creates a TreeSet out of it while RestGetMappingAction modidies the result of this method. The reason why I was looking at this was to figure out whether we should actually have a toSortedSet
method in this class, or whether what we actually needed was a toUnmodifiableSortedSet
.
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.
I pushed e9dd166.
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.
Thanks for aligning the API with how it's used.
This commit replaces the construction of some collections in static initializers with new collection convenience methods that are available now that we have bumped the minimum Java language level to be higher than Java 8.
This commit replaces the construction of some collections in static initializers with new collection convenience methods that are available now that we have bumped the minimum Java language level to be higher than Java 8.