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

Enable github CI for all (including external) PRs #375

Merged
merged 2 commits into from
Nov 7, 2023
Merged

Conversation

soenkehahn
Copy link
Contributor

@soenkehahn soenkehahn commented Nov 7, 2023

Currently, our github CI doesn't run for PRs from forked repos, e.g. for this one:
#349

This hopefully fixes that.

I think we still have to manually approve CI runs from external forks. But that seems sensible for now.

Update: I first tried out on: [push, pull_request], but that means that for internal PRs we always get duplicated runs, since they're both a PR and a push to a local branch. I changed the config to now:

  • Enable CI only for pushes to the main branch, and
  • enable CI for all PRs.

But this means that we won't get CI runs for pushes to branches other then main in this repo, if there's no PR open for that branch. So if you want a CI run, you'd have to open a PR (possibly a draft PR). I think this is acceptable. WDYT?

@soenkehahn soenkehahn marked this pull request as draft November 7, 2023 17:33
@soenkehahn soenkehahn marked this pull request as ready for review November 7, 2023 17:42
@soenkehahn soenkehahn merged commit 8c2d699 into main Nov 7, 2023
28 checks passed
@soenkehahn soenkehahn deleted the sh/ci-on-prs branch November 7, 2023 18:05
@sol
Copy link
Contributor

sol commented Nov 7, 2023

Currently, our github CI doesn't run for PRs from forked repos, e.g. for this one: #349

This hopefully fixes that.

I think we still have to manually approve CI runs from external forks. But that seems sensible for now.

Update: I first tried out on: [push, pull_request], but that means that for internal PRs we always get duplicated runs, since they're both a PR and a push to a local branch. I changed the config to now:

  • Enable CI only for pushes to the main branch, and
  • enable CI for all PRs.

But this means that we won't get CI runs for pushes to branches other then main in this repo, if there's no PR open for that branch. So if you want a CI run, you'd have to open a PR (possibly a draft PR). I think this is acceptable. WDYT?

That's basically what I have been doing for most of my repositories for quite a while now, and it works very well for me.

https://github.com/sol/hpack/blob/main/.github/workflows/build.yml#L3-L15

You may also want that first three lines, so that in-progress builds are canceled when you add additional commits to a PR.

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