Skip to content

chore: API Diff converted to GitHub action #22383

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

chore: API Diff converted to GitHub action #22383

wants to merge 7 commits into from

Conversation

Artur-
Copy link
Member

@Artur- Artur- commented Oct 1, 2025

No description provided.

@github-actions GitHub Actions
Copy link

github-actions bot commented Oct 1, 2025 •
edited
Loading

Test Results

1 270 files  ±0  1 270 suites  ±0   1h 17m 4s ⏱️ -8s
8 793 tests ±0  8 726 ✅ ±0  67 💤 ±0  0 ❌ ±0 
9 249 runs  +7  9 172 ✅ +7  77 💤 ±0  0 ❌ ±0 

Results for commit 681182c. ± Comparison against base commit 4b1abc8.

♻️ This comment has been updated with latest results.

@ZheSun88
Copy link
Contributor

ZheSun88 commented Oct 6, 2025

based on this test PR.
#22384

we got +1.0.0 from changing the docs.. this seems not correct

@mcollovati
Copy link
Collaborator

based on this test PR. #22384

we got +1.0.0 from changing the docs.. this seems not correct

1.0.0 is from the Teamcity job, since it cannot determine the base branch from the PR target branch

@mcollovati
Copy link
Collaborator

I updated the branch for the test PR. If everything is working as expected, we should see 0.0.1 then overwritten by vaadin-bot with 1.0.0

@mcollovati
Copy link
Collaborator

Not really. The test PR is comparing using -Dapi.reference.version=api-diff-github-SNAPSHOT, but given that PR should usually run against protected branches, it seems the GH workflow correctly computes the target branch.

@mcollovati
Copy link
Collaborator

To test the main branch -> 25.0-SNAPSHOT combination we can only wait for the PR to be merged.

@ZheSun88
Copy link
Contributor

ZheSun88 commented Oct 6, 2025

okay.. so if Artur is okay.. i think we can merge this PR.. then fix the problem when needed..
we will not disable the TC build for API-diff label for now.. just in case.

mcollovati
mcollovati requested changes Oct 6, 2025
View reviewed changes
ZheSun88 and others added 5 commits October 7, 2025 09:11
@ZheSun88
define the github.event.xxx in env and extract the java.version for e…
5b8d744 
…asier maintenance
@Artur-
Merge remote-tracking branch 'origin/main' into api-diff-github
97becf8
@Artur-
Simplify
782416a
@Artur- @claude
chore: use JAVA_VERSION variable in GitHub workflows
2832514 
Replace hard-coded JDK 21 references with JAVA_VERSION environment variable to improve maintainability and make Java version updates easier.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@Artur- @claude
chore: simplify vaadin-spring directory check in workflow
681182c 
Remove unnecessary conditional check for vaadin-spring directory as it always exists in the repository.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@Artur- Artur- marked this pull request as ready for review October 11, 2025 12:08
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
332 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

mcollovati
mcollovati approved these changes Oct 14, 2025
View reviewed changes
Copy link
Collaborator

@mcollovati mcollovati left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a potential change for a timeout, but not a blocker.

.github/workflows/validation.yml
exit 1
api-diff-labeling:
if: github.event_name == 'pull_request_target'
timeout-minutes: 90
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the timeout can be reduced to 5 or max 10 minutes.

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

Reviewers

@mcollovati mcollovati mcollovati approved these changes

@ZheSun88 ZheSun88 Awaiting requested review from ZheSun88

Assignees

No one assigned

Labels

+0.0.1

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Artur- @ZheSun88 @mcollovati @vaadin-bot @mshabarov