build: merge code formatter CI workflow with the validation build, test only after formatter succeeds #22453
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Artur-
force-pushed
the
flow-spotless-update
branch
2 times, most recently
from
58cd01c to
52aec19
Compare
Artur-
force-pushed
the
flow-spotless-update
branch
from
b217255 to
347f208
Compare
Artur-
force-pushed
the
flow-spotless-update
branch
from
a1b95ee to
347f208
Compare
Merges the separate formatter workflow into the validation workflow with automatic commit and push capabilities for formatting changes. Changes: - Deleted standalone .github/workflows/formatter.yml - Integrated formatter as first job in validation.yml workflow - Added auto-commit functionality using PAT token (VAADIN_BOT_TOKEN) - Formatter now commits and pushes changes when formatting issues detected - Workflow stops after committing, new workflow run is triggered by the commit - On second run, detects bot commit and skips formatting, proceeds to validation - Changed workflow permissions to include contents:write for pushing commits - Build and test jobs only run if formatter succeeds (no changes or after bot commit) Infinite loop prevention: - Checks if last commit author is github-actions[bot] - If true, skips formatting step and proceeds to validation - Prevents endless formatting cycles if formatter is inconsistent Token requirements: - Requires VAADIN_BOT_TOKEN secret with repo and workflow permissions - Workflow fails immediately if token is not configured - PAT needed because GITHUB_TOKEN cannot trigger workflow runs PR comment behavior: - Posts comment when formatting changes are auto-committed - Deletes comment when formatting is correct - Includes list of files that were modified 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Artur-
force-pushed
the
flow-spotless-update
branch
from
ff69418 to
37e8c75
Compare
Artur-
changed the title
|
Just a question: do I understand correctly, that this change makes validation workflow unusable for external contributor PRs, since the bot cannot push to respective branches? If so, would it make sense to introduce a fallback to just checking formatting in the absence of push access? |
|
Most often contributors tend to check the checkbox that maintainers can make updates. Does this mean that formatting will also work? I have not tried |
platosha
changed the title
platosha
requested changes
Oct 13, 2025
.github/workflows/validation.yml
Outdated
| build: | ||
| formatter: | ||
| needs: check-permissions | ||
| if: github.event_name == 'pull_request_target' |
There was a problem hiding this comment.
I think, this condition needs to removed or adjusted. I tried dispatching this workflow from the PR branch manually using the dispatch button, here's what happened:
- it skipped the formatter job because it was not the
pull_request_targetevent, - then it skipped the build job probably because
formatteris required, - then it skipped the testing jobs also,
- ...and then it reported a false positive, oh no!
See: https://github.com/vaadin/flow/actions/runs/18466985120
Keeping the manual dispatch function working is useful for enabling maintainers to verify workflow changes from PR branches. Let's not limit the supported triggers. If auto-commit from the formatter isn't appropriate, it could fallback to checking only.
Integrated latest changes from main while preserving auto-commit formatter: - Kept auto-commit formatter in validation.yml (no separate formatter.yml) - Added JAVA_VERSION and PR_NUMBER environment variables - Added api-diff-labeling job for automatic PR version labeling - Incorporated all code changes from main (signal binding, Abbr component, etc.) - Updated to use env.JAVA_VERSION for JDK setup steps - Kept parallel unit test count at 3 The auto-commit formatter functionality is preserved - formatting issues will be automatically committed to PRs instead of blocking them.
Changes: - Remove restrictive 'if: pull_request_target' condition from formatter job - Make auto-commit steps conditional on pull_request_target event - Add check-only mode that fails on formatting errors for other triggers - Update checkout to work with both PR and dispatch events - Update build job to proceed when formatter is skipped - Make PR comments conditional on pull_request_target This allows maintainers to manually dispatch the workflow from PR branches for testing workflow changes, while preserving auto-commit for PRs and check-only validation for other triggers. Addresses feedback from @platosha
Performance improvements: - Add step to detect changed Java files compared to target branch - Use spotless ratchetFrom feature to only format changed files - Skip formatter entirely when no Java files have changed - Update all dependent step conditions to handle skipped formatter This significantly speeds up the formatter job, especially for: - Large PRs with changes in specific modules - PRs that don't modify Java files (e.g., documentation changes) - Repositories with many files The ratchetFrom feature compares against the PR base branch and only applies formatting to files that have been modified, added, or renamed.
Major performance improvement by detecting which modules contain changed files and running spotless only on those specific modules using Maven's -pl (--projects) flag. Changes: - Detect unique Maven module paths from changed Java files - Pass module list to mvn spotless:apply using -pl flag - Combined with ratchetFrom for maximum efficiency Performance impact: - Before: Maven processes ALL modules (~50+) even for single file change - After: Maven only processes affected modules (typically 1-3) Example scenarios: - Change in flow-server only: formats 1 module instead of 50+ - Change in 3 modules: formats 3 modules instead of 50+ - Nested modules (flow-plugins/flow-maven-plugin) handled correctly This dramatically reduces formatter execution time from minutes to seconds for typical PRs that touch only a few modules.
|
|
Changed so the formatter finds changed files + modules and only runs the formatter there. Speeds it up by a minute or so when only one file is changed |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Merges the separate formatter workflow into the validation workflow with
automatic commit and push capabilities for formatting changes.
Changes:
Infinite loop prevention:
Token requirements:
PR comment behavior: