SEZ9 commented on PR #11500:
URL: https://github.com/apache/seatunnel/pull/11500#issuecomment-6393740529

   Thanks for syncing with `dev`, and thanks for the F1–F6 pointers above. I'll 
check them against the diff at `94209409a6a1635ed8887a0d0615b49295c58bd8` 
myself and resolve whatever checks out; with the net patch unchanged since 
`fe8b1b39c12` apart from the merge and the fork `Build` run `40055334113` green 
on this head, that should be quick.
   
   Two parts of my earlier points are not covered by those pointers yet:
   
   - **F1 (e2e)** — The unit tests cover the missing-key default, but does the 
shared e2e container still copy a `seatunnel.yaml` with `dynamic-slot: true`? 
If so, is it intended for this PR that the e2e suite runs entirely on dynamic 
slots, or should at least one lane exercise the static default (possibly as a 
follow-up)?
   - **F5 (`slot-num: 20`)** — The "(default)" static-slot snippets pair 
`dynamic-slot: false` with `slot-num: 20`, which reads as if 20 were the 
default slot count. Could you either drop `slot-num` from those snippets or add 
a one-line note that it is an example value?
   
   <!-- streview-comment:2028 -->


-- 
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