-
Notifications
You must be signed in to change notification settings - Fork 71
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[upload] Prs get auto-closed when changes are reordered sometimes #170
Comments
Fixing the reordering problem < A < B < C < D D < o Detecting whether changes in a series got reordered (Do this only if any base changed? might be ok to always do it) Equivalently, take prinfo.headref for every review. If that is equal to baseref for any review in relatives, mark that relative review as You can also do the equivalent with only one traversal -- just start at the leaves and maintain a set -- probably not worth extra effort given we don't store leaf info Preventing github destruction when pushing reordered changes
|
We've had a long running issue where github may mark prs as merged or closed when trying to reorder relative series. This is caused by push + update not being atomic (see comments for more details). To work around this we'll - detect it in a dumb way, if 2 or more prs had their base changed this upload. this will result in some false positives, but the only cost is an additional ~1s push that people likely won't notice, and will save us from needing to write more complex logic to detect it correctly. - push a dummy commit to every single branch initially that prevents github from marking any of the prs as merged. - update as normal - do an extra push to remove the dummy commit Topic: reorder4 Reviewers: aaron, brian-k Fixes: #170
We've had a long running issue where github may mark prs as merged or closed when trying to reorder relative series. This is caused by push + update not being atomic (see comments for more details). To work around this we'll - detect it in a dumb way, if 2 or more prs had their base changed this upload. this will result in some false positives, but the only cost is an additional ~1s push that people likely won't notice, and will save us from needing to write more complex logic to detect it correctly. - push a dummy commit to every single branch initially that prevents github from marking any of the prs as merged. - update as normal - do an extra push to remove the dummy commit Topic: reorder4 Reviewers: aaron, brian-k Fixes: #170
We've had a long running issue where github may mark prs as merged or closed when trying to reorder relative series. This is caused by push + update not being atomic (see comments for more details). To work around this we'll - detect it in a dumb way, if 2 or more prs had their base changed this upload. this will result in some false positives, but the only cost is an additional ~1s push that people likely won't notice, and will save us from needing to write more complex logic to detect it correctly. - push a dummy commit to every single branch initially that prevents github from marking any of the prs as merged. - update as normal - do an extra push to remove the dummy commit Topic: reorder4 Reviewers: aaron, brian-k Fixes: #170
This seems to happen when two PRs are in a relative chain and their order is swapped.
I think we can do something tricky where we push a temporary intermediate fake commit to break dependencies then push again to send the real changes. ghstack does something similar here ezyang/ghstack#23
The text was updated successfully, but these errors were encountered: