weiqingy opened a new pull request, #664:
URL: https://github.com/apache/flink-agents/pull/664

   Linked issue: #597
   
   ### Purpose of change
   
   Decouples code-style enforcement from non-style CI jobs. Today every `mvn 
install` / `mvn test` triggers `spotless-maven-plugin` because its `check` goal 
is bound to Maven's `validate` phase (`pom.xml:298-302`). A single style 
violation cascades across UT, IT, and cross-language jobs and masks genuine 
test failures — defeating the purpose of having a dedicated `Code Style Check` 
CI.
   
   After this PR, only the dedicated `Code Style Check` job runs spotless. 
Every other non-lint CI job sets `SKIP_SPOTLESS_CHECK: true` at the job level, 
and `tools/ut.sh` / `tools/build.sh` honor it by appending 
`-Dspotless.skip=true` to every `mvn` invocation.
   
   **Behavior summary**
   
   | Caller | Before | After |
   |--------|--------|-------|
   | Non-lint CI jobs | Run spotless redundantly via `validate` phase | Skip 
spotless |
   | `Code Style Check` CI job | Enforces spotless | Unchanged |
   | Local `tools/ut.sh` / `tools/build.sh` | Run spotless | Unchanged (env var 
unset) |
   | `tools/lint.sh -c` | Run spotless via `mvn spotless:check` | Unchanged |
   
   The skip mechanism reuses `pom.xml:42-43`'s existing `spotless.skip` 
property — the same knob the JDK-21 profile (`pom.xml:122-131`) already uses 
for plugin compatibility.
   
   **Files modified** (4 files, +39 / -5):
   
   - `tools/ut.sh` — Read `SKIP_SPOTLESS_CHECK` env var; append 
`${SPOTLESS_FLAG}` to the 4 mvn invocations
   - `tools/build.sh` — Same pattern; 1 mvn invocation
   - `.github/workflows/ci.yml` — Job-level `env: { SKIP_SPOTLESS_CHECK: true 
}` on 4 jobs (`java_tests`, `python_it_tests`, `java_it_tests`, 
`cross_language_tests`)
   - `.github/workflows/build_wheel.yml` — Same on 3 jobs (`java_tests`, 
`e2e_tests`, `build_python_wheels`)
   
   **Why job-level (not step-level) env**: a job-level placement covers every 
step in the job, including the latent `tools/e2e.sh` fallback path that may 
call `bash tools/build.sh` (`tools/e2e.sh:101, 114`). Step-level fragmentation 
risks missing a second mvn-invoking step in a multi-step job — exactly the bug 
class this PR is meant to prevent.
   
   ### Tests
   
   This is CI-infrastructure-only. No unit tests added (the repo has no 
shell-script test framework; existing scripts like `tools/ut.sh`, 
`tools/build.sh`, `tools/lint.sh` are also untested by any framework).
   
   Verified locally:
   
   - `bash -n tools/ut.sh && bash -n tools/build.sh` — syntax OK.
   - YAML parse via `python3 -c "import yaml; yaml.safe_load(...)"` on both 
workflows.
   - Env-var conditional truth table — only `true` and `1` set the flag; 
`false`, `0`, empty, `TRUE`, `yes` leave it unset.
   - `mvn -pl api validate -Dspotless.skip=true` — emits `[INFO] Spotless check 
skipped`.
   - `mvn -pl api validate` (control) — spotless runs as before.
   - `mvn help:evaluate -Dexpression=spotless.skip -DforceStdout` — `true` with 
`-D`, `false` without.
   - `./tools/lint.sh -c` — `### FORMAT CHECK PASSED ###` (lint script 
unchanged).
   - Grep audit: `lint` job sections in both workflows untouched; 
`tools/lint.sh` does not read `SKIP_SPOTLESS_CHECK`; no env-var name collision 
repo-wide.
   
   Post-merge CI verification: the PR's CI run should show `Code Style Check` 
still executing spotless and every other job's mvn log free of spotless lines.
   
   ### API
   
   No API changes.
   
   ### Documentation
   
   - [ ] `doc-needed`
   - [x] `doc-not-needed`
   - [ ] `doc-included`
   


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