Skip to content
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.

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

Run revapi workflow on workflow/build system changes #10485

Conversation

findepi
Copy link
Member

@findepi findepi commented Jun 11, 2024

a PR updating gradle (#10474) should not have green CI because of the revapi problem outlined here #10476 (comment). run the revapi checks when CI workflows or build system configuration changes

@github-actions github-actions bot added the INFRA label Jun 11, 2024
@findepi
Copy link
Member Author

findepi commented Jun 11, 2024

cc @jbonofre @Fokko

@jbonofre
Copy link
Member

Not sure it makes sense to trigger revapi (api compatibility workflow) when gradle or github folders are updated.

Can you please provide the rationale here ?

The problem with Gradle 8.2+ update is not when revapi is triggered, it's an issue on the revapi gradle plugin itself (see palantir/gradle-revapi#612). I'm working to fix this issue in the plugins (both revapi and shadow) and I have a fallback PR with a revapi script.

So I don't think it makes sense to change the workflow trigger.

@findepi
Copy link
Member Author

findepi commented Jun 11, 2024

thank you @jbonofre for your review and feedback!

Not sure it makes sense to trigger revapi (api compatibility workflow) when gradle or github folders are updated.

Can you please provide the rationale here ?

what if i change the revapi (api compatibility workflow) itself? without any change, the build won't trigger, so we won't know if the changed workflow works

what if i change something in the build system (e.g. gradle version) that breaks revapi? without any change, the build won't trigger, so we won't know if the build system change impacted the revapi workflow.

(of course, this change doesn't safeguard against silent problems like described here #10368 (comment))

@jbonofre
Copy link
Member

I understand your point but not sure it would happen. Revapi just checks the Java modules. I'm not against your change, I just wonder if it actually fix/improve something 😃

@findepi
Copy link
Member Author

findepi commented Jun 11, 2024

It doesn't fix anything, because nothing is currently broken. The change is supposed to make it less likely that things break in the future.

@findepi findepi force-pushed the findepi/run-revapi-workflow-on-workflow-build-system-changes-ba6d4c branch from ebb300a to c58c6d7 Compare June 17, 2024 13:59
@findepi findepi requested a review from nastra June 18, 2024 08:23
@nastra nastra merged commit 08cc776 into apache:main Jun 18, 2024
3 checks passed
@findepi findepi deleted the findepi/run-revapi-workflow-on-workflow-build-system-changes-ba6d4c branch June 18, 2024 12:44
@findepi
Copy link
Member Author

findepi commented Jun 18, 2024

thanks for the merge!

jasonf20 pushed a commit to jasonf20/iceberg that referenced this pull request Aug 4, 2024
sasankpagolu pushed a commit to sasankpagolu/iceberg that referenced this pull request Oct 27, 2024
zachdisc pushed a commit to zachdisc/iceberg that referenced this pull request Dec 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants