Migrate ReferenceDocs resource to plain text#113866
Migrate ReferenceDocs resource to plain text#113866DaveCTurner merged 4 commits intoelastic:mainfrom
ReferenceDocs resource to plain text#113866Conversation
Removes the dependency on `XContent` parsing so we can move this out of `:server` and into `:libs:core`.
DaveCTurner
added
>non-issue
:Core/Infra/Core
|
Pinging @elastic/es-core-infra (Team:Core/Infra) |
|
I've previously used csv for the releaseversions mapping files - I think csv is a bit more obvious that the structure matters than whitespace? |
|
CSV is more painful to edit by hand (or in IntelliJ) than plain text. The parsing is very strict, it will be obvious that the structure matters pretty quickly if you get it wrong. |
|
Is it that much more painful? Here, it's just a comma between the two fields. The number of spaces is somewhat arbitrary, and could cause spurious parsing errors. And csv is consistent with the other text storage files we've used |
The spacing is not arbitrary and the parsing errors are not spurious - the strictness in this area is 100% deliberate. CSV is not totally trivial to parse (it supports quoted strings, and escaped commas, etc etc) so I'd want to defer that work to a library instead of hand-rolling it. But the whole point of this change is to eliminate dependencies from this code so we can move it to |
|
Also note that we are expecting the Docs build to parse this file independently, and that's written in Perl; this format is very easy to parse with Perl's regexes whereas a CSV (with quotes and escaping) would add complexity there too. |
|
Ah, I wasn't going with full CSV, I was just going with a comma |
|
Yeah I'd rather have spaces here. It matters less when all the values are symbols of almost the same length. |
rjernst
left a comment
rjernst
left a comment
There was a problem hiding this comment.
LGTM, one nit but I don't feel very strongly about it.
| BOOTSTRAP_CHECK_CLIENT_JVM bootstrap-checks-client-jvm.html | ||
| BOOTSTRAP_CHECK_USE_SERIAL_COLLECTOR bootstrap-checks-serial-collector.html | ||
| BOOTSTRAP_CHECK_SYSTEM_CALL_FILTER bootstrap-checks-syscall-filter.html | ||
| BOOTSTRAP_CHECK_ONERROR_AND_ONOUTOFMEMORYERROR bootstrap-checks-onerror.html |
There was a problem hiding this comment.
The max length for padding seems like it will get overrun. Can we start with larger padding to limit having to reformat this file in the future?
There was a problem hiding this comment.
Fair point. Increased to 64 because that ought to be enough for anybody ;)
💔 Backport failed
You can use sqren/backport to manually backport by running |
Removes the dependency on `XContent` parsing so we can move this out of `:server` and into `:libs:core`. Backport of elastic#113866 to `8.x`
Removes the dependency on `XContent` parsing so we can move this out of `:server` and into `:libs:core`.
* Add link to CircuitBreaker exception message * Account #113866 * Update docs/changelog/113561.yaml
Removes the dependency on `XContent` parsing so we can move this out of `:server` and into `:libs:core`. Backport of #113866 to `8.x`
Removes the dependency on `XContent` parsing so we can move this out of `:server` and into `:libs:core`.
…ic#113561) * Add link to CircuitBreaker exception message * Account elastic#113866 * Update docs/changelog/113561.yaml
4 participants
Removes the dependency on
XContentparsing so we can move this out of:serverand into:libs:core.