ESQL: Rename Validatable iface to PostLogicalOptimizationVerificationAware#119985
Conversation
Rename interface to align it with the similar post-analysis interfaces.
bpintea
added
>refactoring
auto-backport
elasticsearchmachine
added
the
Team:Analytics
|
Pinging @elastic/es-analytical-engine (Team:Analytics) |
costin
left a comment
costin
left a comment
There was a problem hiding this comment.
LGTM. The only comment is around naming - PostLogicalOptimizationVerificationAware is very explicit but also long; I wonder if we should drop the Logical prefix and assume it's explicit since I don't expect a PostPhysicalOptimizationVerificationAware interface any time soon.
bpintea
added
the
auto-merge-without-approval
…Aware (elastic#119985) Rename interface to align it with the similar post-analysis interfaces. Related elastic#119798.
💚 Backport successful
|
| * </pre> | ||
| * | ||
| */ | ||
| void postLogicalOptimizationVerification(Failures failures); |
There was a problem hiding this comment.
The method name should be aligned with the class name (including in the javadoc):
postLogicalOptimizationVerification --> postOptimizationVerification
| package org.elasticsearch.xpack.esql.capabilities; | ||
|
|
||
| import org.elasticsearch.xpack.esql.common.Failures; | ||
| import org.elasticsearch.xpack.esql.expression.function.grouping.Bucket; |
There was a problem hiding this comment.
Move the class package into the javadoc or better yet find a different example - it's best to not extend unnecessary packages in the documentation since it unnecessarily ties this interface to Bucket.
…Aware (elastic#119985) Rename interface to align it with the similar post-analysis interfaces. Related elastic#119798.
3 participants
Rename interface to align it with the similar post-analysis interfaces.
Related #119798.