RFR: 1797: Add support for /backport pull request command

[Thomas Stüfe]

Hi Erik :)

On Thu, Feb 2, 2023 at 12:33 PM Erik Duveblad <erik.helin at oracle.com> wrote:

> > On 2 Feb 2023, at 09:28, Thomas Stüfe <thomas.stuefe at gmail.com> wrote:
> >
> > > 1. If the `/backport` command is used in an **open** pull request, it
> adds a label `Backport=repo:branch` to the PR. If the PR is later
> integrated, the PR bot scans for backport labels and begins the backporting
> process. To cancel the backport, the user can use `/backport disable repo
> master` to remove the label before the PR is integrated.
> >
> > This does not sound desirable. Fixes should undergo some bake time in
> > the primary repo before they get backported anywhere. With the current
> > situation it is up to the person doing the backport to go to the commit
> > in question and know that the commit has been suitable tested/baked.
> >
> > David
> > -----
> >
> >
> > I agree with David. Making it easy to downport fresh changes right away
> is not something we should encourage.
>
> Please do not jump to conclusions about what Zhao is proposing without
> understanding the patch :) Zhao’s patch is only making it easier to create
> a backport _pull request_, it will not automatically create any backport
> commits.
>
> A user can already today type `/backport` in a commit comment for the
> commit a pull request resulted in once it was integrated. This can be done
> only seconds after the pull request was integrated and the Skara bots will
> then go ahead and create a “backport pull request” (see the Skara wiki on
> backports for more details [0]). Zhao’s patch only removes the manual step
> of having to navigate to the GitHub page for the resulting commit and add a
> commit comment. With Zhao’s patch a user can now instead type `/backport`
> in the pull request and the Skara bots will create the “backport pull
> request” once the pull request has been integrated.
>
> So, this patch is not creating any new workflows, it is just removing a
> couple of manual steps needed today to create a "backport pull request".
> The "backport pull request" should of course not be integrated until the
> commit in mainline has received proper “soak time” and testing.
>
>
Sure, I got all that.

But auto-creating a backport branch (and the backport PR too?) after a
patch had been committed makes only sense if you plan to push the backport
in the immediate future. Because if you don't, if you let it sit for a
month, the backport branch will be stale. Even if the stale branch is still
mergeable, one should still rebase it to the top of the backport repo in
order to test it correctly. Since the backport repo may have changed in the
meantime and may now be incompatible with your patch, even though the patch
is technically mergeable.

And my point was that this should be quite rare, and limited to very
important or very simple patches. So simplifying this process may be an
indication that we do something often that we should not do often.

But I don't oppose this change if you think it is fine.

Cheers, Thomas

p.s. there is one detail I did not understand:

"With Zhao’s patch a user can now instead type `/backport` in the pull
request and the Skara bots will create the “backport pull request” once the
pull request has been integrated."

Today, /backport does not create a pull request, or? It only creates the
backport branch; the pull request creation is still a manual step - either
a button press or a manual process via git. So, will /backport in an open
PR behave differently from that and create the PR right away?


> Thanks,
> Erik
>
> [0]: https://wiki.openjdk.org/display/SKARA/Backports
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/skara-dev/attachments/20230202/5eaa4152/attachment.htm>