-
Notifications
You must be signed in to change notification settings - Fork 46
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
ci: only trigger on pushes to main branches #196
Conversation
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.
How about version branches?
The trigger is the limitation of Github itself and I would accept the current state.
It is actually cause only when you have access to the repo.
If someone really don't want to trigger the push
event, you can actually Fork and PR
.
Could add something like |
Yes, we always use that pattern. Maybe just |
Doesn't appear to like the curly braces, and it's not one of the allowed values in filter list. A suggested alternative on that page is |
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.
lgtm
Just nagging here: |
Sorted, just using 'v*' now. |
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.
LGTM
Worst case scenaria if somebody creates a branch "very-good" is that the pipeline runs twice. I think this is acceptable.
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 am not sure if it worth.
The push
CI is run on the branch and the pull request
CI is run on pull request.
Which means, every new branch will not be run if it does not send PR.
No, CI test on feature branch.
I am not blocked to land this, it just worth it or not. Will someone actually use feature-branch
and check CI?
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 it worth trying this configuration.
We don't push code into the repo without PR, except the bump version one, so I don't see blockers here.
What I like most about this PR (if it works) is that we don't waste resources (electricity, logs, disk, network)
Merging
What's been the general consensus on this? Worth adding to other repos? |
See fastify/workflows#56.
This will stop PRs from running the CI workflows twice (one for the PR and one for the push to the PR), which will cut down run times for Dependabot PRs that wait for both to complete before merging, and the time waiting for all checks to complete for normal PRs (see https://github.com/fastify/fastify-helmet/pull/195/checks).
If happy with this then i will apply to rest of the repos.
Checklist
and the Code of conduct