-
Notifications
You must be signed in to change notification settings - Fork 121
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
chore(ci): run CI on all branches and v2 pull requests #744
Conversation
push: | ||
branches: [main] | ||
pull_request: | ||
branches: [main] | ||
branches: ["*"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing pull_request
means that external pull requests are no longer run against the CI. Its unlikely that this is wanted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ohhhh I was not aware, thanks for pointing it out. I'll revert this part of the changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reverting would make the CI run twice on internal PQs tho. There's no perfect way out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd expect that the rules collectively determine whether or not CI runs, rather than determining the number of times that identical jobs are started. Are you sure about this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please see #749
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Holy crap, you were right. Wtf
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one runs the branch, one merges the branch into main and then runs the CI. When PR aren't on top of the main this might be different → different meaning → two jobs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wojpawlik I think I have fixed it now, the trade-off is that jobs will only be duplicated when we merge the v2 branch via a PQ, which only happens once. Can you TAL?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one merges the branch into main and then runs the CI
Nope. Pushing to a PQ that wants to merge A into B triggers push
on A and pull_request
on B.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we are talking past each other. You talk about what triggers them, I talk about what runs and why GitHub doesn’t deduplicate them.
pull_request
merges with the target branch (in most cases main) before running the CI. (And doesn’t run when a merge can’t be done automatically.) push
does not do merging. So they have different goals with different meaning.
This lets us view CI when working on #675.