chore(build): Use pull request triggers #9771
Conversation
Use pull request triggers as the primary mechanism for testing pull requests, rather than push triggers. Github workflows have a characteristically very low limit on the number of concurrent MacOS jobs, no matter the subscription level, and has a limit of 50 non-macOS jobs. Combined with the number of jobs that we run, this greatly limits the velocity of merging and checking code. However, the number of jobs that we run often represents duplicate work - our test/lint workflows run on both pull_request (targeting the predicted merge branch that github automatically creates) and on push (targeting whatever obj/ref was just pushed). By using pull_request triggers and limiting push to release, we can greatly cut down on the amount of work we do, while still testing the same code. The hole that's left is running tests on branches that developers have pushed but do not want to open a pull request for, or that has a merge conflict that prevents pull request checks from automatically running. In this case, we have workflow_dispatch triggers so that the checks can be run through the web UI, and we should probably add magic-comment triggers for a similar thing.
Codecov Report
@@ Coverage Diff @@
## edge #9771 +/- ##
=======================================
Coverage 75.10% 75.10%
=======================================
Files 1980 1980
Lines 52282 52282
Branches 5044 5044
=======================================
Hits 39265 39265
Misses 12051 12051
Partials 966 966
Flags with carried forward coverage won't be shown. Click here to find out more. |
| @@ -6,12 +6,16 @@ name: 'API test/lint/deploy' | |||
| on: | |||
| # Most of the time, we run on pull requests, which lets us handle external PRs | |||
| pull_request: | |||
| types: [opened, reopened, synchronize, ready_for_review] | |||
There was a problem hiding this comment.
If I'm reading the docs correctly, this is equal to the default types, plus the addition of ready_for_review. What does adding ready_for_review do / why do we want to do that? Why is the default insufficient?
There was a problem hiding this comment.
what i'd hope is that it adds a trigger when you take a pr from draft to ready, but it might not really add much you're right
| - os: 'macos-latest' | ||
| python: '3.7' |
There was a problem hiding this comment.
Why are we excluding this? Since this is what many of us are running locally, isn't this one of the important ones to have in CI so we don't break each other's dev environments?
There was a problem hiding this comment.
I'd like to exclude one of them at least - macos runners are the harsher limit. i figured 3.10 was more likely to be what users would be on, but if we want to prioritize dev envs we can exclude 3.10 again (or, sigh, keep both)
There was a problem hiding this comment.
Hm, that's a good point.
🤷
There was a problem hiding this comment.
Nightly runs of all versions might be a good solution for this issue
|
Note on this PR - for |
Use pull request triggers as the primary mechanism for testing pull requests, rather than push triggers. Github workflows have a characteristically very low limit on the number of concurrent MacOS jobs, no matter the subscription level, and has a limit of 50 non-macOS jobs. Combined with the number of jobs that we run, this greatly limits the velocity of merging and checking code. However, the number of jobs that we run often represents duplicate work - our test/lint workflows run on both pull_request (targeting the predicted merge branch that github automatically creates) and on push (targeting whatever obj/ref was just pushed). By using pull_request triggers and limiting push to release, we can greatly cut down on the amount of work we do, while still testing the same code. The hole that's left is running tests on branches that developers have pushed but do not want to open a pull request for, or that has a merge conflict that prevents pull request checks from automatically running. In this case, we have workflow_dispatch triggers so that the checks can be run through the web UI, and we should probably add magic-comment triggers for a similar thing.
Use pull request triggers as the primary mechanism for testing pull
requests, rather than push triggers.
Github workflows have a characteristically very low limit on the number
of concurrent MacOS jobs, no matter the subscription level, and has a
limit of 50 non-macOS jobs. Combined with the number of jobs that we
run, this greatly limits the velocity of merging and checking code.
However, the number of jobs that we run often represents duplicate work
predicted merge branch that github automatically creates) and on
push (targeting whatever obj/ref was just pushed). By using pull_request
triggers and limiting push to release, we can greatly cut down on the
amount of work we do, while still testing the same code.
The hole that's left is running tests on branches that developers have
pushed but do not want to open a pull request for, or that has a merge
conflict that prevents pull request checks from automatically running.
In this case, we have workflow_dispatch triggers so that the checks can
be run through the web UI, and we should probably add magic-comment
triggers for a similar thing.