Skip to content

ci: run spotless on every pull request - #591

Merged
wangyb-A merged 1 commit into
mainfrom
fix/spotless-all-modules
Aug 4, 2026
Merged

ci: run spotless on every pull request#591
wangyb-A merged 1 commit into
mainfrom
fix/spotless-all-modules

Conversation

@wangyb-A

@wangyb-A wangyb-A commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Problem

spotless-check had a paths filter covering only sdk/**, sdk-testing/**, sdk-integration-tests/**, and examples/**, but mvn spotless:check --file pom.xml validates every Maven module. Modules missing from that filter — otel-plugin, conformance-tests, coverage-report — can merge unformatted code without the job ever running, and that code then fails the check on unrelated pull requests.

This has already happened twice:

Commit PR Paths touched Gate ran? Spotless
14ad139 #578 otel-plugin/, examples/ yes (matched examples/**) clean
acfa484 #583 otel-plugin/ only no fails

#583 has no spotless-check entry in its check-runs at all. An earlier instance under conformance-tests/** was fixed by #586.

Changes

  • drop the paths filter so the job runs on every PR targeting main
  • add push on main so drift is caught if anything else bypasses the gate
  • add workflow_dispatch for manual runs
  • reformat the two otel-plugin test files that are unformatted on main today

The Java changes are formatting-only: with all whitespace stripped, both files are byte-identical to main (Palantir split long constructor argument lists across lines).

Validation

  • mvn -B -q spotless:check --file pom.xml — passes (fails on main today)
  • mvn -pl otel-plugin test — 145 passed
  • workflow YAML parses; triggers are pull_request, push, workflow_dispatch
  • diff contains exactly the three intended files

Tradeoff

Removing the filter means every PR runs this job, including docs-only changes. The alternative — listing all seven modules — stays targeted but silently rots the next time a module is added, which is the root cause here.

@wangyb-A
wangyb-A marked this pull request as ready for review August 4, 2026 21:57
@wangyb-A
wangyb-A requested a review from a team August 4, 2026 21:57
@wangyb-A
wangyb-A temporarily deployed to ai-pr-review-runtime August 4, 2026 21:57 — with GitHub Actions Inactive
@wangyb-A
wangyb-A temporarily deployed to ai-pr-review-runtime August 4, 2026 21:57 — with GitHub Actions Inactive
@wangyb-A
wangyb-A merged commit 8f799c6 into main Aug 4, 2026
17 of 20 checks passed
@wangyb-A
wangyb-A deleted the fix/spotless-all-modules branch August 4, 2026 21:59
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

Claude AI review

No actionable findings.

The PR contains only:

  • .github/workflows/check-spotless.yml — removes the paths filter and adds push (main) and workflow_dispatch triggers. This is correct: mvn spotless:check --file pom.xml validates every Maven module, so a path filter could skip the job for changes it still checks. Making it always run on PRs is safer and, if the job is a required status check, avoids the branch-protection stall a conditionally-run required check can cause. The docs-only-PR tradeoff is acknowledged and is not a defect.
  • otel-plugin/.../ExecutionOtelPluginTest.java and otel-plugin/.../InvocationOtelPluginTest.java — whitespace-only reformatting of OperationEndInfo(...) constructor calls. I confirmed each split call preserves argument order, count, and values (the CANCELLED and TIMED_OUT cases), so there is no behavioral, typing, or serialization change.

Residual test risk: minimal. The Java edits are formatting-only and don't alter or add test logic. The workflow change is not exercised by unit tests, but its YAML is straightforward and its effect (running Spotless more broadly) can only surface pre-existing formatting violations, not mask regressions.

Reviewed commit ecc3df8d96fa30157696f8969ae1dd12d7fed8f4. Workflow run

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

Codex AI review

No actionable findings.

Residual risk: Per review constraints, Maven/Spotless and the updated GitHub Actions triggers were not executed; validation remains dependent on CI.

Reviewed commit ecc3df8d96fa30157696f8969ae1dd12d7fed8f4. Workflow run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants