ci: run github actions on pull requests

[alxndrsn]

Closes #1308

There is some subtlety here:

• to allow personal development branches, exceptions can be included. As @ktuite names branches consistently, I've included that exception here. For other contributors, they could pick a naming scheme and add to the config
• i don't think soak tests need to run on every PR, so they're restricted to master, and also can be triggered manually from https://github.com/getodk/central-backend/actions this change has been moved to ci: only run soak tests on master branch #1320 to avoid discussions getting entangled

What has been done to verify that this works as intended?

It's not always 100% possible to test github actions config without merging it.

Why is this the best possible solution? Were any other approaches considered?

See #1308 for alternative suggestions(s).

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

No.

Does this change require updates to the API documentation? If so, please update docs/api.yaml as part of this PR.

No.

Before submitting this PR, please make sure you have:

• run make test and confirmed all checks still pass OR confirm CircleCI build passes
• verified that any code from external sources are properly credited in comments or that everything is internally sourced

[lognaturel]

Won't this end up with duplicate runs for the branches that are marked as exceptions? Why not one of the alternatives in the issue?

[alxndrsn]

Won't this end up with duplicate runs for the branches that are marked as exceptions?

Yes

Why not one of the alternatives in the issue?

I think the approach taken in this PR is an improvement on the current state. It's also quick to implement, and should be easy to understand how it affects job runs and how to modify configuration if it's not acceptable.

Alternatives suggested in the issue:

skip-duplicate-actions Action

This looks very powerful, and also potentially useful for more complicated build configurations in future.

Example PR: #1322

job conditionals

This looks like a blunt instrument, and will presumably trigger actions but not individual jobs within them.

Example PR: #1321

---

I think we just live with the double runs like we do with pyxform.

Double-runs could also be avoided if this repo is not used for individuals' development branches.

Example PR without branch restrictions: #1324

---

I'll open alternative PRs for the different approaches and see how they look.

As mentioned above, it may not be possible to properly test each approach without merging changes to master.

I've opened example PRs against a fork of this repo and shared screenshots & links to show how they behave.

[alxndrsn]

With this PR on master, and another PR opened against it from ktuite/... (alxndrsn#25):

PR View

Actions View

[alxndrsn]

After running through test PRs with the different configuration options, #1324 looks like the cleanest solution (simplest code, least clutter in actions & PR checks list). It has one downside, which is that branches pushed to getodk/central-backend without open PRs will not have actions run for them.

[lognaturel]

branches pushed to getodk/central-backend without open PRs will not have actions run for them.

And to forks as well, right? Right, that's the current behavior.

Let's start with that and if we find that we miss having runs on push we can take out the branch restriction and accept the double runs as we do for pyxform and other repos.

[alxndrsn]

And to forks as well, right? Right, that's the current behavior.

No, I think your struck-out comment is correct! I think this will change:

from current:

• fork branches:
  • ✅ no PR: runs once per push
  • ❌ PR open: does not run (on PR-target-repo; still runs once-per-push on source-repo)
• local branches:
  • ✅ no PR: runs once per push
  • ✅ PR open: runs once per push

to (#1324)

• fork branches:
  • ❌ no PR: does not run
  • ✅ PR open: runs once per push
• local branches:
  • ✅ PR not open: runs once per push
  • ✅ PR open: runs once per push

[alxndrsn]

I'm now thinking this is the best compromise:

on:
  push:
  pull_request:

which should give us:

• fork branches:
  • ✅ no PR: runs once per push
  • ✅ PR open: runs once per push
• local branches:
  • ✅ no PR: runs once per push
  • ⚠️ PR open: runs twice per push

Update: I've opened #1335 which implements this approach.

[alxndrsn]

Closing in favour of #1335