Avoid extraneous CI runs

[obycode]

Our ci.yml currently includes the following trigger:

  pull_request_review:
    types:
      - submitted

This causes the CI to be rerun whenever a review is submitted. This is not needed and causes unnecessary delays and runner usage. The jobs are already guaranteed to be re-run when anything changes based on the pull_request triggers:

  pull_request:
    types:
      - opened
      - reopened
      - synchronize
      - ready_for_review

[wileyj]

i'm kind of thinking here that we should only trigger on

• synchronize
• ready_for_review

i'm going to try a few options here, then PR against next

[obycode]

I think I would lean toward "opened" and "synchronize". Removing "opened" and keeping "ready_for_review" would not allow us to check the test status before asking for reviews, and that seems like a good practice. Unless we're really concerned about using too much runner time on "not ready" PRs, then I could understand that change.

[wileyj]

I think I would lean toward "opened" and "synchronize". Removing "opened" and keeping "ready_for_review" would not allow us to check the test status before asking for reviews, and that seems like a good practice. Unless we're really concerned about using too much runner time on "not ready" PRs, then I could understand that change.

yea, i see where you're coming from and that's a good point. no reason to review if the expected tests are failing

[wileyj]

@obycode i found an interesting edge case that might cause some problems without further changes.

consider the case where a branch is PR'ed where the only change is to CHANGELOG.md or some other non-code file (currently configured to ignore md and yml files).
In this scenario, CI will never run - but the status checks are still enabled which would require either a small "code" change (like an extra comment/line in a file), or an override to merge a PR (which i woild prefer avoiding).

i'm not sure if there's a way to disable status checks based on what is changed in the PR, leaving the only other option being that we trigger workflows on any PR change, markdown files included.

any thoughts on this?

[wileyj]

actually, i think i have my answer - it's possible now, but will have to come in a future PR. for now i'll simply remove the ignored paths trigger:

    paths-ignore:
      - "**.md"
      - "**.yml"

[wileyj]

next: #4764
develop: #4765
master: #4766