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.

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

chore(ci): run CI on all branches and v2 pull requests #744

Merged
merged 1 commit into from
Jan 20, 2025
Merged

chore(ci): run CI on all branches and v2 pull requests #744

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions .github/workflows/deno.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,7 @@ name: grammY

on:
push:
branches: [main]
pull_request:
branches: [main]
branches: ["*"]
Comment on lines 12 to +13
Copy link
Member

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.

Copy link
Member Author

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

Copy link
Contributor

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.

Copy link
Member Author

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?

Copy link
Member Author

Choose a reason for hiding this comment

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

Please see #749

Copy link
Member Author

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

Copy link
Member

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.

Copy link
Member Author

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?

Copy link
Contributor

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.

Copy link
Member

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.


jobs:
backport:
Expand Down
Loading