RFR: 1663: Mark integrated Pull Requests as properly merged in GitHub repositories

Julian Waters jwaters at openjdk.org
Thu Nov 3 15:13:34 UTC 2022


On Thu, 3 Nov 2022 13:34:11 GMT, Erik Joelsson <erikj at openjdk.org> wrote:

> 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.

My mistake- I forgot to create the issue and enable the test workflows, both have now been done.

I've since reverted the changes to PreIntegrations, which should resolve the problem of concurrent changes to the targetRef in question. The only other solution I can think of though is saving the targetRef before the pr/ branch swap happens, and then immediately resetting it back to the saved instance after the merge in `markIntegratedAndMerged` (and if really required, also printing the branch the Pull Request was integrated into in the "pushed as commit" message). I unfortunately currently have no way of testing if GitHub even allows doing this however, and documentation doesn't really seem to mention anything about this either

> 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.

That's a bit of a bummer, though maybe that can be circumvented with a few `instanceof GitHubPullRequest`s? The change is, after all, only actually implemented for GitHub

-------------

PR: https://git.openjdk.org/skara/pull/1409


More information about the skara-dev mailing list