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

[GR-60823] Do not run GraalVM gate on master branch. #10415

Merged
merged 1 commit into from
Jan 6, 2025

Conversation

graalvmbot
Copy link
Collaborator

Currently, the GraalVM gate runs on every push of a PR, and whenever we push the master and release branches. Since all changes to the master branch are proposed through PRs, all PRs are at least tested twice: on every push (last commit of PR merged into master) and after merge (merge commit created by our merge bot).

This PR removes this duplication to reduce the use of CI resources.

@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Jan 6, 2025
@graalvmbot graalvmbot closed this Jan 6, 2025
@graalvmbot graalvmbot deleted the fniephaus/GR-60823/graalvm-gate branch January 6, 2025 23:05
@graalvmbot graalvmbot merged commit 9542667 into master Jan 6, 2025
13 checks passed
@zakkak
Copy link
Collaborator

zakkak commented Jan 7, 2025

@fniephaus one thing to keep in mind is that when testing a PR, the base branch commit hash (master or release/* in this case) might not be the same as when you actually merge the PR. I.e. more changes might have been merged between the CI run and the actual merge.

@fniephaus
Copy link
Member

@zakkak do you think this is an issue in practice?

@zakkak
Copy link
Collaborator

zakkak commented Jan 8, 2025

@zakkak do you think this is an issue in practice?

It depends on how important it is to ensure that master is never broken.

If you are OK with having master broken for a short period of time, since the breakage will probably be noticed in the first couple of PRs that will be submitted after the breaking merge, then it's an acceptable compromise.

A major drawback is that while master is broken people working on it might spend time trying to understand what they are doing wrong although the error won't be on their side.

In general, I personally don't expect such issues to pop up often and am personally OK with the change, just wanted to make clear that this change might result in master getting broken despite the CI being green in the PRs.

@fniephaus
Copy link
Member

A major drawback is that while master is broken people working on it might spend time trying to understand what they are doing wrong although the error won't be on their side.

Right, this shouldn't happen at all because we still run our internal gates but if it happens in practice, we can add master builds again.

In general, I personally don't expect such issues to pop up often and am personally OK with the change, just wanted to make clear that this change might result in master getting broken despite the CI being green in the PRs.

Good, and please let us know if you ever see something like this happening.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants