Skip to content

Skip push CI for most branches #564

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Skip push CI for most branches #564

wants to merge 1 commit into from

Conversation

djc
Copy link
Member

@djc djc commented Apr 28, 2025

My impression is that the common practice of running CI for both push and pull_request can be pretty expensive/slow while adding almost no value. Restrict branch CI to main and branches prefixed with rel- or ci/.

@djc djc requested review from cpu and ctz April 28, 2025 13:57
@@ -5,6 +5,7 @@ permissions:

on:
push:
branches: ['main', 'rel-*', 'ci/*']
pull_request:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
pull_request:
pull_request:
workflow_dispatch:

This would mean, in the case where some maintainer really does want to run CI for something without opening a PR, they can do that by pushing the branch and then running it manually (this enables the UI for that).

Copy link
Member

Choose a reason for hiding this comment

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

Manually invoking each CI job for each branch push is a very unappealing workflow to me :-/

@cpu
Copy link
Member

cpu commented Apr 28, 2025

Restrict branch CI to main and branches prefixed with rel- or ci/.

Do you plan to do this in the other repos, e.g. rustls/rustls? I think it would be valuable to do this consistently, at least in the magic prefix/suffix to get it to happen for a branch push intentionally. E.g. rustls-platform-verifier is using *_dev, can we standardize on that?

I personally don't find it too expensive/slow (especially with cancel-in-progress), and I do appreciate being able to run CI easily before opening a PR so I can make sure it's green before notifying anyone. On the other hand, needing to know a magic suffix/prefix per-repo is annoying and I end up duplicating my branches with different names and having to check the .yml file.

@djc
Copy link
Member Author

djc commented Apr 28, 2025

Restrict branch CI to main and branches prefixed with rel- or ci/.

Do you plan to do this in the other repos, e.g. rustls/rustls? I think it would be valuable to do this consistently, at least in the magic prefix/suffix to get it to happen for a branch push intentionally. E.g. rustls-platform-verifier is using *_dev, can we standardize on that?

Would @ctz's suggestion of adding workflow_dispatch alleviate your concerns? Sounds like that might be helpful.

@cpu
Copy link
Member

cpu commented Apr 28, 2025

Would @ctz's suggestion of adding workflow_dispatch alleviate your concerns? Sounds like that might be helpful.

I don't think it would. It's even more friction than having to name your branch correctly IMO because you can't say "run all the workflows" you have to do it both per-push, and per-workflow. The drop-down to select the branch to run is also a bit annoying in that it doesn't sort by recently pushed (If I remember right).

I'm not strictly opposed to using the branch name approach if I'm in the rough here, but I would prefer it be consistent between rustls org repos if we can swing it.

@djc
Copy link
Member Author

djc commented Apr 29, 2025

Do you plan to do this in the other repos, e.g. rustls/rustls? I think it would be valuable to do this consistently, at least in the magic prefix/suffix to get it to happen for a branch push intentionally. E.g. rustls-platform-verifier is using *_dev, can we standardize on that?

Yes, I can probably do this for the other repos -- agreed on the value of consistency. Do you prefer using *_dev? I like that ci/ is a little more explicit about the goal (and I sort of generally prefer / or - over _, which might be silly), but don't feel too strongly (and given that I just open PRs directly my workflow wouldn't be impacted).

@cpu
Copy link
Member

cpu commented Apr 29, 2025

Do you prefer using *_dev? I like that ci/ is a little more explicit about the goal

No strong preference here. ci/ seems OK if you like that one. I don't think anyone will mind a change from *_dev on the rustls-platform-verifier repo, and I don't think we restrict CI by branch name in any other org repos presently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants