oscerd commented on PR #27425:
URL: https://github.com/apache/camel/pull/27425#issuecomment-6013323509

   Some follow-up points from a review that was still in progress when this was 
merged (checked against 7e6db68a57). The first two give wrong guidance on 
realistic input:
   
   1. **`doCatch`/`doFinally` next to `doTry` are reported as separate EIPs.** 
`stepNames()` includes `doCatch` and `doFinally`, which are also options of 
`doTry`, and `stepWithTwoKeys` tests `steps.contains(k)` before 
`options.contains(k)`. So a mis-indented
      ```yaml
      - doTry:
          steps:
            - to: "direct:x"
        doCatch:
          - exception: [java.io.IOException]
      ```
      gets `doCatch: is another EIP: start it as its own item, - doCatch:`. 
Following that gives a standalone `- doCatch:` step, which the schema accepts, 
but `CatchReifier` then fails at start-up (`This doCatch should have a doTry as 
its parent`), and `FinallyReifier` does the same for `doFinally`. Testing 
`options.contains(k)` first fixes it. The only other key that is both an option 
and an EIP is claimCheck's `filter`, where the option is a string and the EIP 
an object.
   2. **An option next to a short-form EIP is told to indent under a scalar.** 
For `- log: "hello"` followed by `loggingLevel: WARN` at the item's column (or 
`- to: sql:...` followed by `parameters:`), the hint says to indent the key 
under `log:`. A scalar has no options, so that is a YAML error, and 
`parseError` then sends the key back to its original column (quoted value) or 
to an outer mapping such as `from:` (plain value). That brings back the loop 
this PR wants to stop. When `node.get(eip)` is a value node, the hint could say 
to write the EIP as a map instead, e.g. `log: {message: "hello", loggingLevel: 
WARN}`. `STRING_STEP_PROPERTY` already maps each short form to its property.
   3. **`TOP_KEY` also matches single-word prose headings.** 
`^([A-Za-z][\w-]*):\s*$` matches `Explanation:`, `Notes:` or `Summary:` 
appended after the routes, which then get "is a top-level key ... write it as a 
list item" instead of the "is not YAML, put explanations in a # comment" 
message. Checking the key against the schema's top-level entries would keep 
`errorHandler:`, `beans:` and the rest.
   4. Minor: the user-facing message ends with `(CAMEL-25372)`, the only 
message string in the validator carrying a Jira key. It's printed to users and 
models, and could move to the comment above it.
   
   _Claude Code on behalf of oscerd. This review was generated by an AI agent 
and may contain inaccuracies. Please verify all suggestions before applying. It 
does not replace specialized review tools or static analysis._
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to