feat: allow PRs to be made with automerge enabled #88
Conversation
amonkhouse
force-pushed
the
auto-merge
branch
from
ae3a76f to
919e09d
Compare
amonkhouse
force-pushed
the
auto-merge
branch
from
919e09d to
a2da37e
Compare
|
@mc-jones I've tested this a few times and it's quite flaky, sometimes the call to enable auto merge fails as the PR is in a "clean" state, plus sometimes not all the checks trigger before auto merge is enabled and it merges early. |
Is your sense that we're just inherently limited by the native functionality, or we need to code a more elegant solution? |
|
@mc-jones hopefully just that we need to code a more elegant solution, but I'm not fully convinced it will work. It seems you can only mark a PR as squash/merge on green when there are checks that are running. That would be easy enough to handle, but I also had an issue where one check was running and the PR got marked as merge on green, but the check finished before any others started and it merged when it should have actually ran another 4 or so checks... |
| "compilerOptions": { | ||
| /* Basic Options */ | ||
| "target": "es5" /* Specify ECMAScript target version: 'ES3' (default), 'ES5', 'ES2015', 'ES2016', 'ES2017', 'ES2018', 'ES2019', 'ES2020', or 'ESNEXT'. */, | ||
| "target": "es2019" /* Specify ECMAScript target version: 'ES3' (default), 'ES5', 'ES2015', 'ES2016', 'ES2017', 'ES2018', 'ES2019', 'ES2020', or 'ESNEXT'. */, |
There was a problem hiding this comment.
I added a file for the graphql queries and mutations as they made the functions harder to read. But this is just a preference.
There was a problem hiding this comment.
Similarl to the dedicated graphql-quries.ts file, I added a dedicated type file. Not everyone likes to break them out into their own file but I feel like this improved readability a bit. Can revert if we disagree.
| }) | ||
|
|
||
| status = response?.data?.repository?.pullRequest?.statusCheckRollup?.state ?? null | ||
| await new Promise((resolve) => setTimeout(resolve, 1000)) // wait for 1 second |
There was a problem hiding this comment.
There could still be some race conditions here. For example, a status check could start and finish during the 1 second timeout. We would break out of the loop but we would miss our opportunity to enable auto merge, which would be non-obvious to developers. If we see this happening we can reduce the timeout period.
Adds new logic flow to PR creation:
automergeboolean arg toupdateRepo, which defaults to false (we could make this true)createAndMergePullRequestto return PR node ID, needed for call to enable auto mergeautomergeis true, once the PR has been created, we submit a GraphQL call to enable auto mergehttps://artsyproduct.atlassian.net/browse/PHIRE-2629