KQL plugin - Code cleanup #116180
KQL plugin - Code cleanup #116180
Conversation
elasticsearchmachine
added
needs:triage
| @@ -0,0 +1,12 @@ | |||
| module org.elasticsearch.kql { | |||
There was a problem hiding this comment.
| @@ -17,7 +17,7 @@ base { | |||
|
|
|||
| dependencies { | |||
| compileOnly project(path: xpackModule('core')) | |||
| compileOnly "org.antlr:antlr4-runtime:${versions.antlr4}" | |||
| api "org.antlr:antlr4-runtime:${versions.antlr4}" | |||
There was a problem hiding this comment.
api instead of compileOnly
There was a problem hiding this comment.
Using api will expose this dependency to other modules. Is that needed? 🤔
There was a problem hiding this comment.
Honestly, this is the same way other plugins using ANTLR runtime (lang-painless, ql, and esql-core) include it, so I didn’t think too much about it.
Using api probably makes sense for ql and esql-core since they’re extended, while implementation should work fine for kql and lang-painless
Sorry, I saw your comment only after I merged the PR but I can change api to implementation in a follow-up if you want.
There was a problem hiding this comment.
An even better patch would be to add the ANTLR runtime to libs. Maybe something for later.
| @@ -211,15 +211,15 @@ private static boolean isEscapedKeywordSequence(String input, int startIndex) { | |||
| if (startIndex + 1 >= input.length()) { | |||
| return false; | |||
| } | |||
| String remaining = Strings.toRootLowerCase(input.substring(startIndex)); | |||
| String remaining = input.substring(startIndex).toLowerCase(Locale.ROOT); | |||
There was a problem hiding this comment.
Removing a useless dependency to org.apache.logging.log4j.util.Strings
afoucret
added
:Search/Search
elasticsearchmachine
added
Team:Search
|
Pinging @elastic/es-search (Team:Search) |
|
Hi @afoucret, I've created a changelog YAML for you. |
💚 Backport successful
|
Few code cleanup for the KQL modules.