RFR: Mark integrated Pull Requests as properly merged in GitHub repositories
erikj at openjdk.org
Thu Nov 3 13:37:39 UTC 2022
On Tue, 1 Nov 2022 16:55:16 GMT, Julian Waters <jwaters at openjdk.org> wrote:
> Skara currently closes integrated Pull Requests, since the actual integration is done internally and then pushed to the repository separately. This makes every integrated request always look like it was closed to an outside observer, forcing the use of a special integrated flag to distinguish between integrated and closed, or rejected and closed. On top of that, it just looks awful in the interface all around (and is also not included in the accepted Pull Request count by GitHub for the committer, which is frustrating for newer contributors to a surprising extent). In the [willful absence of a reply from the GitHub team to allow marking a pull request as merged through a flag](https://github.com/orgs/community/discussions/12437), and GitLab being unlikely to make a similar change either, a workaround with minimal impact can be used instead: Every pull has a corresponding pre-integration branch created at the time it was made, and this special branch is marked for deletion when the
pull is integrated. Following the principle of waste not, want not, a change that is currently only implemented for GitHub repositories instead resets the pre-integration branch to the source ref of the pull request, then merges the request into it's own pre-integration branch before the actual deletion occurs, leaving minimal impact on most of Skara's infrastructure and with virtually no effect on already existing repositories (and it also keeps the aesthetically pleasing integrated flag!).
> - [x] Properly handle all dependencies of the pre-integration branch before it's modified irreversibly
> - [ ] Change test code to match modified integration logic (if required)
First of all, for a change as big as this, a bug is definitely needed. It should really be filed first and explain what and why you intend to change.
This is an interesting idea, but I'm not sure its worth the tradeoff of losing the information about which branch a PR was integrated into. We already have multi branch development happening in some repos and more of that is coming.
At first glance this also looks brittle. You need to understand that the bots are designed to be interruptible at any point in time, restarted and being able to continue. The integration logic and order is carefully crafted to function this way. As an example, assuming that the second to last target targetRef change is the one we actually integrated into could be false if someone else was changing that around the time of integration. There are a lot of corner cases that would need careful testing before this change could be accepted, and once it went live, I would expect at least some fallout.
We are currently blocked from using pr/X branches in GitLab due to [SKARA-1173](https://bugs.openjdk.org/browse/SKARA-1173), so this proposed change would be blocked until that is resolved regardless.
More information about the skara-dev