JuliaCI / PkgTemplates.jl

Create new Julia packages, the easy way
https://juliaci.github.io/PkgTemplates.jl
MIT License
639 stars 101 forks source link

CI plugins: skip PR builds for non-forks #193

Open christopher-dG opened 4 years ago

christopher-dG commented 4 years ago

There was a bit of discussion on Slack yesterday about getting GitHub Actions to skip PR builds for branches that are already being built for push events: it's redundant to run your test suite twice for every push when you have an open PR. This annoys me a bunch too, so maybe it's a sensible default to give our CI config templates.

The GitHub Actions config looks like this:

if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name != github.repository)
DilumAluthge commented 4 years ago

I would prefer that this not be the default behavior.

As I posted on Slack, my preferred workflow is to not run CI on pushes to branches that aren't master:

name: CI

on:
  pull_request:
    branches:
      - master
  push:
    branches:
      - master
    tags: '*'
christopher-dG commented 4 years ago

I'd argue that your preference is more niche than the proposed one 😅 I still don't know that I want either of these yet, just putting it up for consideration.

DilumAluthge commented 4 years ago

😂

In Travis, my anecdotal experience is that people are more likely to use my preference.

I think each workflow has its use case. No need to impose either on the user.

That being said, the two workflows are not compatible with each other. If you enable both of them, then you won't be running CI on non-fork pull requests.

So I think the best option is to not modify the default behavior. Then we could have each of these two workflows available on an opt-in basis by using optional keyword arguments.

nickrobinson251 commented 2 years ago

i think this might be resolved by https://github.com/invenia/PkgTemplates.jl/pull/325 (/ https://github.com/invenia/PkgTemplates.jl/issues/323) so new pushes to a PR cancels any on-going build, but i might not be understanding this issue 😊 is there some other behaviour that we should introduce here (whether the default or not)?