-
Notifications
You must be signed in to change notification settings - Fork 25.1k
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
ESQL autogenerate docs v3 #124312
ESQL autogenerate docs v3 #124312
Conversation
* More links fixed (support more automatic conversion of asciidoc links to markdown) * Apostrophes manually fixed (used ot be done by asciidoc, but not now by MD) * New TERMS function added with hidden docs * More standard use of back-ticks for code in docs
Pinging @elastic/es-docs (Team:Docs) |
Pinging @elastic/es-analytical-engine (Team:Analytics) |
ac2cd7b
to
f95947c
Compare
The reformatting broken more than it fixed.
And moving forward it feels risky to change error messages in this subtle way
…elasticsearch into esql_autogenerate_docs_v3
This makes it easier to separate test code from docs generation code, and also separate functions from operators.
It seems when we use the JSON builder, the UTF8 is escaped.
…g names as keys in OPERATORS map.
…elasticsearch into esql_autogenerate_docs_v3
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.
Looks good!
There's a comment around the apostrophes formatting that I think could be some kind of misformatting 👀
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.
As it's not detected as a rename, I suppose we'll keep both for now, right? (The Kibana files) And remove the old later. Or did we remove them already?
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 sure I follow. I only see one abs.json
file.
@@ -12,7 +12,7 @@ | |||
* <h2>Guide to adding new aggregate function</h2> | |||
* <ol> | |||
* <li> | |||
* Aggregation functions are more complex than scalar functions, so it's a good idea to discuss | |||
* Aggregation functions are more complex than scalar functions, so it’s a good idea to discuss |
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.
Are those apostrophe changes intended? I've seen them changed in some functions too
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.
This was very intentional, and quite a lot of work too.
The older asciidoc system converted these when publishing HTML, while the new MD system does not, so we need to do the conversion ourselves, manually. Most text editors also auto-convert these when you type, so any documentation written using editors will have the correct apostrophe character, but documentation generated from Java code will not (java editors do not auto-convert single-quotes to apostrophes when used in text).
@@ -210,7 +154,7 @@ protected static List<TestCaseSupplier> anyNullIsNull( | |||
* | |||
* Also, if this was the first time we saw the signature we copy it | |||
* *again*, replacing the argument with null, but annotating the | |||
* argument's type as `null` explicitly. | |||
* argument’s type as `null` explicitly. |
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.
Same here; it looks like some auto-formatting?
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.
As before, it was me doing manual reformatting because the auto-formatting was no longer working.
knownFiles = Map.ofEntries( | ||
entry("esql-commands", "esql/esql-commands.md"), | ||
entry("esql-enrich-data", "esql/esql-enrich-data.md"), | ||
entry("esql-examples", "esql/esql-examples.md"), | ||
entry("esql-functions-operators", "esql/esql-functions-operators.md"), | ||
entry("esql-implicit-casting", "esql/esql-implicit-casting.md"), | ||
entry("esql-metadata-fields", "esql/esql-metadata-fields.md"), | ||
entry("esql-multivalued-fields", "esql/esql-multivalued-fields.md"), | ||
entry("esql-process-data-with-dissect-grok", "esql/esql-process-data-with-dissect-grok.md"), | ||
entry("esql-syntax", "esql/esql-syntax.md"), | ||
entry("esql-time-spans", "esql/esql-time-spans.md"), | ||
entry("esql-limitations", "esql/limitations.md"), | ||
entry("esql-function-named-params", "esql/esql-syntax.md"), | ||
entry("query-dsl-query-string-query", "query-dsl-query-string-query.md") | ||
); |
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 it's fine for now, but I would reevaluate migrating those in the annotations.
Otherwise it's harder to know how to make a new link, as you have to find this logic here.
Depending on how many usages are of such links, and how longer they really get, it may not be that terrible (?)
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 agree, but it is kind-of a line in the sand. I took an approach that seemed to achieve two things:
- Minimize my work (and therefor hopefully others)
- Keep the annotation text clean and readible
- Make backporting easier (fewer conflicts in the java code that have these annotations)
But it is true that it is less clear now how to create links. I think we should keep this approach for now, but re-evaluate this going forward. I see two directions:
- We pick and publish a standard limited set of short-links for convenience, make sure everyone knows about them, and re-format the rest
- We move to the new formatting entirely, removing all these short links
💔 Backport failed
You can use sqren/backport to manually backport by running |
Building on the work started in elastic#123904, we now want to auto-generate most of the small subfiles from the ES|QL functions unit tests. This work also investigates any remaining discrepancies between the original asciidoc version and the new markdown, and tries to minimize differences so the docs do not look too different. The kibana json and markdown files are moved to a new location, and the operator docs are a little more generated than before (although still largely manual).
Building on the work started in elastic#123904, we now want to auto-generate most of the small subfiles from the ES|QL functions unit tests. This work also investigates any remaining discrepancies between the original asciidoc version and the new markdown, and tries to minimize differences so the docs do not look too different. The kibana json and markdown files are moved to a new location, and the operator docs are a little more generated than before (although still largely manual).
… ES (#214321) ## Summary Based on the changes here elastic/elasticsearch#124312 changes the paths of the ES docs and definitions
… ES (elastic#214321) ## Summary Based on the changes here elastic/elasticsearch#124312 changes the paths of the ES docs and definitions
Building on the work started in #123904, we now want to auto-generate most of the small subfiles from the ES|QL functions unit tests.
This work also investigates any remaining discrepancies between the original asciidoc version and the new markdown, and tries to minimize differences so the docs do not look too different.
The kibana json and markdown files are moved to a new location, and the operator docs are a little more generated than before (although still largely manual).
Tasks remaining:
applies_to
for appropriate features (difference between 9.1 and 9.0)*-new.md
,*-orig.md
and intermediate functions/*md, etc.