dev: Trim back the overgrown CI jobs

[victorlin]

Same reasoning as @tsibley in nextstrain/cli@fab709a:

Running on push and PRs is often redundant. For PRs, we really care
about the putative merge of the PR branch, and that's what "on:
pull_request" tests. We typically do not need push-level CI results for
PRs. On the other hand, CI results for every push to master are nice to
have both as a safety backstop and for the linear chain of CI history it
produces (e.g. to debug the impact of external changes on our CI).

The primary downside I see is that you can no longer push without
opening a PR just to see what CI says, but I think that's an acceptable
tradeoff, especially now that draft PRs are a thing. To mitigate this
downside, "on: workflow_dispatch" allows CI to be manually dispatched
for a specific branch/tag/commit if you really don't want to open even
a draft PR.

Trimming unnecessary CI jobs reduces the time to completion for CI runs
(good for the dev loop) and reduces organization-level job queuing,
which can negatively impact the workflows of other repos.

Noting that another downside is not having push-triggered CI runs on forks, but workflow_dispatch and draft PRs are good alternatives.

[corneliusroemer]

I have semi-recently discovered that there's an additional important downside not mentioned in @tsibley's commit message. I think it's good to note this for the record:

• CI doesn't run on: "pull_request" if there are merge conflicts. New pushes on the PR branch that will then not receive any CI feedback, potentially misleading devs who are looking out for red crosses as signs something is off (while there's a lack of green check mark, at least personally I'm more attuned to red crosses than green ticks and can easily miss lack of green ticks).

[joverlee521]

CI doesn't run on: "pull_request" if there are merge conflicts.

Huh, that's good to know. I see now that it's noted in the GH Action docs for the pull_request event:

Workflows will not run on pull_request activity if the pull request has a merge conflict. The merge conflict must be resolved first.

I was curious about the reason for this limitation, found in discussion:

For pull_request event, the related ref is a fake merged branch: refs/pull/:prNumber/merge, however it’s not related to actions/checkout. If there’re any conflicts, github cannot create it automatically for the workflow.

[corneliusroemer]

Yep, there are some additional interesting side effects: CI can pass on PR, then a commit happens on main (CI for it passes), but upon merge CI fails due to some interaction of the recent new main commit that didn't manifest as a merge conflict.

On pull request should ideally rerun CI whenever there's a new commit on the target branch, but I don't think it does (haven't tested but would be surprised if I was wrong).

The same issue will of course happen with the "on: push" strategy, because there you don't test on the new changes you haven't merged (or rebased onto).

[victorlin]

On pull request should ideally rerun CI whenever there's a new commit on the target branch, but I don't think it does (haven't tested but would be surprised if I was wrong).

Yeah, there are no events to trigger the workflow upon update of target branch. There is a setting to reduce the effects of this: Require branches to be up to date before merging. That would mean setting up branch protection rules and status check requirements. I'm not opposed to it and would appreciate the benefits, but this would be a change in merging strategy that should get a 👍 from all devs.