[Documentation] Update xtokenize-language.adoc - #25977
Conversation
- Clarify distinction between XML tokenize (xtokenize) and convential tokenize. - Add need Maven dependency - Add example
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
davsclaus
left a comment
There was a problem hiding this comment.
Thanks for taking the time to clarify the xtokenize vs. tokenize distinction and add a worked example — the motivation and the Java/XML examples are accurate and a real improvement over the previous one-liner. Two things need to be addressed before this can merge:
- Catalog mirror not regenerated. Editing
components/camel-stax/src/main/docs/xtokenize-language.adocalso requires regenerating and committing the mirrored copy atcatalog/camel-catalog/src/generated/resources/org/apache/camel/catalog/docs/xtokenize-language.adoc(see this project's documentation conventions). I checked.github/workflows/pr-build-main.ymlon this branch: the build step runs./etc/scripts/regen.shand then fails if there are any uncommitted changes afterward, so this PR is very likely to fail CI as-is. Runningmvn -DskipTests install(or the project's regen script) incomponents/camel-staxand committing the regenerated catalog file should fix it. - YAML example uses invalid syntax — see inline comment below.
Separately, not a blocker but worth confirming: this PR targets camel-4.22.x directly rather than main. This project's usual flow lands changes on main first and backports to LTS branches afterward, so main and the other maintenance lines (camel-4.18.x, camel-4.14.x) won't pick up this clarification unless it's ported separately. Was that intentional, or would you be open to retargeting/duplicating this against main?
This review does not replace CodeRabbit, Sourcery, SonarCloud, or other specialized review/static-analysis tools.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Co-authored-by: Claus Ibsen <claus.ibsen@gmail.com>
|
ah the split doc was wrong, fixing that now |
This example was derived from the split EIP (https://camel.apache.org/components/4.22.x/eips/split-eip.html) so then it's also incorrect on that page.
That's OK, but I just use the Edit button on the documentation, this automatically takes that branch. I'm kind of expecting that when I'm using the edit button that branch (latest) is automatically selected. I will try to remember next time. Note that I left the PR open to be edited by the maintainers, so please feel free to change anything, when something is not correct yet. |
|
There are uncommitted changes |
|
you need to make the same change in that other file, or copy it over |
|
I add the same changes to the mirrored copy at catalog/camel-catalog/src/generated/resources/org/apache/camel/catalog/docs/xtokenize-language.adoc |
* Update xtokenize-language.adoc - Clarify distinction between XML tokenize (xtokenize) and convential tokenize. - Add need Maven dependency - Add example * Apply suggestion from @davsclaus Co-authored-by: Claus Ibsen <claus.ibsen@gmail.com> * Update xtokenize-language.adoc --------- Co-authored-by: Claus Ibsen <claus.ibsen@gmail.com>
gnodet
left a comment
There was a problem hiding this comment.
Good documentation improvement that adds Maven dependency info, clarifies the xtokenize vs tokenize distinction, and provides working examples in Java, XML, and YAML DSLs. The examples are technically correct and cover all three DSLs well.
Minor nit: there's a trailing whitespace on line 18 of the doc file ("...XML Tokenize: xtokenize "), but this is purely cosmetic and not blocking.
📋 PR Metadata
| Aspect | Current | Suggested |
|---|---|---|
| Milestone | (none) | 4.22.1 |
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of Guillaume Nodet
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 10 tested, 28 compile-only — current: 9 all testedMaveniverse Scalpel detected 38 affected modules (current approach: 9).
|
This change: