Does not close Restyled PR on merge if base branch changed #150
Labels
Comments
|
/cc @mjgpy3 |
pbrisbin
added a commit
that referenced
this issue
Oct 21, 2021
Given a PR with head pb/fix, we'd locate a Restyled PR by looking for
one where:
base == pb/fix &&
head == restyled/pb/fix
This is a very precise search but fails if GitHub has automatically
switches the base after pb/fix was merged, as it so helpfully does.
We could update this to,
(base == pb/fix || base == {default branch}) &&
head == restyled/pb/fix
But that seems complicated, and would miss a case of a Restyling on a
branch-from-branch situation.
We could simplify to,
head == restyled/pb/fix
But false-positives are scary, considering they could result in a
force-push over a user's unrelated Pull Request.
Therefore, we'll do
head == restyled/pb/fix &&
user =~ ^restyled-io
Having the check on user will prevent force-pushes onto users no matter
how well this works (or not).
Fixes #150.
pbrisbin
added a commit
that referenced
this issue
Oct 21, 2021
Given a PR with head pb/fix, we'd locate a Restyled PR by looking for
one where:
base == pb/fix &&
head == restyled/pb/fix
This is a very precise search but fails if GitHub has automatically
switches the base after pb/fix was merged, as it so helpfully does.
We could update this to,
(base == pb/fix || base == {default branch}) &&
head == restyled/pb/fix
But that seems complicated, and would miss a case of a Restyling on a
branch-from-branch situation.
We could simplify to,
head == restyled/pb/fix
But false-positives are scary, considering they could result in a
force-push over a user's unrelated Pull Request.
Therefore, we'll do
head == restyled/pb/fix &&
user =~ ^restyled-io
Having the check on user will prevent force-pushes onto users no matter
how well this works (or not).
Fixes #150.
pbrisbin
added a commit
that referenced
this issue
Oct 22, 2021
Given a PR with head pb/fix, we'd locate a Restyled PR by looking for
one where:
base == pb/fix &&
head == restyled/pb/fix
This is a very precise search but fails if GitHub has automatically
switches the base after pb/fix was merged, as it so helpfully does.
We could update this to,
(base == pb/fix || base == {default branch}) &&
head == restyled/pb/fix
But that seems complicated, and would miss a case of a Restyling on a
branch-from-branch situation.
We could simplify to,
head == restyled/pb/fix
But false-positives are scary, considering they could result in a
force-push over a user's unrelated Pull Request.
Therefore, we'll do
head == restyled/pb/fix &&
user =~ ^restyled-io
Having the check on user will prevent force-pushes onto users no matter
how well this works (or not).
Fixes #150.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
(Originally reported as https://github.com/restyled-io/restyled.io/issues/246)
When we handle a PR event and find it's Closed, we intend to clean up any open Restyle PRs. We do this by finding a PR where its
baseis theheadof the originating PR. The head/base refs are the only way to search for PRs in the GitHub REST API.GitHub recently launched a feature where if you open a PR-from-PR, then merge the first one, it adjusts the base branch of the second automatically. This breaks the above flow.
Consider the following:
pb/fix->mainrestyled/pb/fix->pb/fixpb/fixrestyled/pb/fix->mainbase: pb/fix, finding none(4) and (5) are a race. Sometimes it works (reverse from above), but usually we lose, which leaves the Restyle PR open.
Possible solutions
We could record the Restyle PR details somewhere when we open it
Our Backend service launches a Restyler process and records stdout/stderr. There is no structured way for Restyler to report back data like this. We would have to build that entire idea to make this possible. Something like GitHub's workflow commands in the stdout stream?
We could look harder for this PR in the GitHub API
The "List PR" endpoint only supports looking by head/base. So using that would require listing all PRs against the original base branch of the originating PR (e.g.
main) and then filtering in code (e.g. bytitle = Restyle {title}).Does the GraphQL API offer more options?
The text was updated successfully, but these errors were encountered: