weiqingy commented on PR #1007: URL: https://github.com/apache/flink-agents/pull/1007#issuecomment-5306020694
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ COMMENT #1 — RESOLVED (PR #1007 top-level, wenjin272: scope -f to e2e only, reject -f without -e, fix the examples) Copy everything below this line ↓ ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ Thanks, that is a better contract. Done in 32a90b02. `-f` now selects the Flink version the e2e tests run against and nothing else, and passing it without `-e` exits 1 with an error naming the remedy. The warning is gone, since the case it warned about no longer exists. Nothing in the repo invokes `-f` without `-e`, so no CI job or documented command changes. The one thing it removes is `./tools/ut.sh -p -f 2.2`, running the Python unit tests against a non-default PyFlink, which nothing here uses. Examples fixed. There were two more carrying the same problem that I had missed: the inline `Examples: -f 2.3, -f 1.20, -f 2.3 -f 1.20` line inside the `-f` block, and the missing-argument hint `(e.g., -f 1.20)`. All three are `-e -f` forms now, and a test asserts the script demonstrates no `-f` form it would itself reject, so a future example cannot reintroduce one. One correction on the wording while implementing it. "Both Java and Python UTs would then consistently use the root POM version" turns out not to hold for Java. The `dist/flink-*` modules stay in the unit-test reactor, since only the two e2e modules are excluded, and each of them pins its own version. `dist/flink-1.20` is the only dist module with a test, and it runs at 1.20.5. So the help text now says the unit tests cannot be retargeted by `-f`, and that each Java module builds against the `<flink.version>` its own pom resolves, rather than naming one version for both lanes. -- 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]
