From 321136c0e96f614c12ce38e01f1b2a1aeb6e1924 Mon Sep 17 00:00:00 2001 From: Mouhcine Aitounejjar Date: Wed, 3 Sep 2025 15:04:50 -0400 Subject: [PATCH 1/2] Update the guide to add a new ESQL function - Update the list of doc files that get generated upon successful unit test run of the new ESQL function - Update the command to generate full docs locally, and reflect the fact it doesn't need Docker anymore - Mention the need to update the expected ESQL functions count in 60_usage.yml - Other minor changes --- .../function/scalar/package-info.java | 60 ++++++++++++------- 1 file changed, 39 insertions(+), 21 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/package-info.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/package-info.java index 8fa3cc03aeed9..5392f8e9d67bd 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/package-info.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/package-info.java @@ -77,7 +77,7 @@ *. To make it work with IntelliJ, also click {@code Build->Recompile 'FunctionName.java'}. * Please commit the generated evaluator before submitting your PR. *

- * NOTE: The function you copied may have a method annotated with + * NOTE 1: The function you copied may have a method annotated with * {@link org.elasticsearch.compute.ann.ConvertEvaluator} or * {@link org.elasticsearch.compute.ann.MvEvaluator} instead of * {@link org.elasticsearch.compute.ann.Evaluator}. Those do similar things and the @@ -151,7 +151,7 @@ *

  • * Now it’s time to generate some docs! * Actually, running the tests in the example above should have done it for you. - * The generated files are + * Make sure to commit the following generated files * * - * Make sure to commit them. Add a reference to the - * {@code docs/reference/query-languages/esql/_snippets/functions/layout/myfunction.md} in the function list - * docs. There are plenty of examples on how - * to reference those files e.g. if you are writing a Math function, you will want to - * list it in {@code docs/reference/query-languages/esql/functions-operators/math-functions.md}. + * In order to make your function appear in the docs, you should add a reference to it in the most suitable two md files under the + * {@code docs/reference/query-languages/esql/functions-operators} and {@code docs/reference/query-languages/esql/_snippets/lists} + * directories. + *

    + * + * For example, if you are writing a Math function, you will want to add it in + * {@code docs/reference/query-languages/esql/functions-operators/math-functions.md} and in + * {@code docs/reference/query-languages/esql/_snippets/lists/math-functions.md}. + *
    *

    * You can generate the docs for just your function by running * {@code ./gradlew :x-pack:plugin:esql:test -Dtests.class='*SinTests'}. It’s just @@ -178,21 +183,16 @@ * } *

    • *
  • - * Build the docs by cloning the docs repo - * and running: + * Install the docs-builder binary from https://github.com/elastic/docs-builder, + * then build the docs locally by running the command below from the elasticsearch directory: * 
    {@code
- * ../docs/build_docs --doc docs/reference/index.md --open --chunk 1
+ * docs-builder serve
  *          }
    - * from the elasticsearch directory. The first time you run the docs build it does a bunch - * of things with docker to get itself ready. Hopefully you can sit back and watch the show. - * It won’t need to do it a second time unless some poor soul updates the Dockerfile in the - * docs repo. *
    • *
  • - * When it finishes building it'll open a browser window. Go to the - * functions page to see your - * function in the list and follow it’s link to get to the page you built. Make sure it - * looks ok. + * You can now browse the docs at http://localhost:3000. Or you can go directly to + * ES|QL functions and operators to see your + * function in the list and follow its link to get to the page you built. Make sure it looks ok. *
    • *
  • * Let’s finish up the code by making the tests backwards compatible. Since this is a new @@ -204,15 +204,33 @@ * to all of your csv-spec tests. Run those csv-spec tests as integration tests to double * check that they run on the main branch. *

    - * **Note:** you may notice tests gated based on Elasticsearch version. This was the old way + * NOTE: You may notice tests gated based on Elasticsearch version. This was the old way * of doing things. Now, we use specific capabilities for each function. *
    • *
  • + * Sometimes we want to implement a function without releasing it. For example, there's a probability its implementation might + * need to change because we're not sure how to handle some edge cases, or we just require further feedback on it. In such case, + * we can still ship it as a snapshot function, and ensure the following: + * + * + * + * When it comes to the correct syntax, e.g.: annotations, groupings, etc., feel free to look at existing snapshot functions and + * Git history. + * + * Depending on whether the new function requires a snapshot build, you may need to increment the expected ES|QL functions count in + * one or both places in {@code x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/60_usage.yml}. You can also run + * these tests locally, just make sure to pass the right flags so you can toggle between snapshot and non-snapshot tests. + *
    • + *
  • * Open the PR. The subject and description of the PR are important because those'll turn * into the commit message we see in the commit history. Good PR descriptions make me very * happy. But functions don’t need an essay. - *
    • - *
  • + * * Add the {@code >enhancement} and {@code :Analytics/ES|QL} tags if you are able. * Request a review if you can, probably from one of the folks that github proposes to you. *
    • From 64d74f3ec240c16fb1820811c5d2c85c9ee4d5f8 Mon Sep 17 00:00:00 2001 From: Mouhcine Aitounejjar Date: Thu, 4 Sep 2025 12:13:16 -0400 Subject: [PATCH 2/2] split the long line to fix checkstyle violation --- .../xpack/esql/expression/function/scalar/package-info.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/package-info.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/package-info.java index 5392f8e9d67bd..cea2da88264e3 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/package-info.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/package-info.java @@ -215,7 +215,8 @@ * *