On 06/22/21 17:38, Kinney, Michael D wrote: > Hi Laszlo, > > I am trying the following configuration that is very conservative: > > actions: > queue: > method: rebase > rebase_fallback: none > name: default > > The auto rebase only attempts a strict rebase. If that attempt at a > strict rebase fails then it will show that there is a conflict that > the developer must take care of. > > I believe any combination of 2 PRs that have overlapping diff stat > should fail a strict rebase. The following link describes the method > and rebase_fallback settings in the queue command. > > https://docs.mergify.io/actions/queue/#id2 > > I would be more concerned if we used a method of merge or a > rebase_fallback of merge. > > Are there examples you can think of where the diff stat overlap and > the strict rebase will succeed?
I've read the strict rebase definition and the above link in the mergify documentation, but I'm none the wiser. Consider the following test case (with master @ 7471751a4d81): git checkout -b b1 master git am b1.patch # attached git checkout -b b2 master git am b2.patch # attached git branch b2-rebase b2 git rebase b1 b2-rebase Locally, this produces the following message for me: > First, rewinding head to replay your work on top of it... > Applying: world > Using index info to reconstruct a base tree... > M ReadMe.rst > Falling back to patching base and 3-way merge... > Auto-merging ReadMe.rst The rebase succeeds and produces the expected result, but that result is *exactly* what a human should review. I don't know if mergify catches the above. While the rebase succeeds locally, it should not succeed in mergify. Using the "git rebase -i" (interactive) command, which uses a different rebase backend (based on git-cherry-pick, not on git-am), and specifying a single "pick" command, the rebase still succeeds; this time without producing any diagnostic messages even. So from an auto-rebase perspective, it's even less desirable. Thanks Laszlo > > Another option to consider is to define an additional 'auto-rebase' label > that is > off by default to enable the auto rebase feature. By default the PR must be > synced > with head when submitted. Only if a maintainer sets the 'auto-rebase' label > will > an auto-rebase be attempted. > > I also want to make it easy for non-maintainers to submit PRs and get CI test > results. > So auto rebase may be useful for that use case. Perhaps the 'auto-rebase' > label > can be considered when the 'push' label is also set. > > Thanks, > > Mike > >> -----Original Message----- >> From: Laszlo Ersek <ler...@redhat.com> >> Sent: Tuesday, June 22, 2021 8:17 AM >> To: Kinney, Michael D <michael.d.kin...@intel.com>; devel@edk2.groups.io; >> spbro...@outlook.com; a...@kernel.org >> Cc: Peter Grehan <gre...@freebsd.org>; Ard Biesheuvel >> <ardb+tianoc...@kernel.org>; Justen, Jordan L >> <jordan.l.jus...@intel.com>; Sean Brogan <sean.bro...@microsoft.com>; >> Rebecca Cran <rebe...@bsdio.com> >> Subject: Re: [edk2-devel] [PATCH] OvmfPkg/Bhyve: clean up TPM_ENABLE remnants >> >> On 06/17/21 23:53, Kinney, Michael D wrote: >>> Hi Sean, >>> >>> Mergify had added a queue feature to handle the rebases automatically and >>> make sure >>> CI passes in the order that the PRs are being applied to the base branch. >> >> I'm opposed to *unconditional* auto-rebase. >> >> On one hand, it is indeed unreasonable to require a human to manually >> rebase a "ShellPkg/Application/AcpiViewApp" series just because a series >> for "SecurityPkg/FvReportPei" was merged a bit earlier. In other words, >> merge requests for unrelated modules should not block each other. >> >> On the other hand, auto-rebase is a bad idea if both series modify at >> least one module in common (especially if both series modify at least >> one *file* in common). In case there is a contextual conflict, even if >> the conflict can be auto-resolved, and even if that resolution >> *compiles*, it has to be reviewed by a human first. >> >> I regularly use the git-range-diff command for this. >> >> At Red Hat we've seen obscure bugs due to silent mis-merges (not in edk2 >> -- in different packages); such issues are difficult to debug. >> >> Bisectability helps for sure, but only if the community treats >> bisectability with high priority in the first place. (That is, if every >> contributor builds their patch set at every stage, before submitting it >> for review.) >> >> Can we restrict the auto-rebase feature to such merge requests whose >> cumulative diffstats do not intersect? >> >> Thanks, >> Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#76997): https://edk2.groups.io/g/devel/message/76997 Mute This Topic: https://groups.io/mt/83497624/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
From 04970caf98f8deeecec2d0bc8808cd8bd34cc530 Mon Sep 17 00:00:00 2001 From: Laszlo Ersek <ler...@redhat.com> Date: Wed, 23 Jun 2021 16:19:25 +0200 Subject: [PATCH] hello --- ReadMe.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/ReadMe.rst b/ReadMe.rst index 8f5db11281bf..30f890ef532e 100644 --- a/ReadMe.rst +++ b/ReadMe.rst @@ -3,6 +3,7 @@ EDK II Project ============== A modern, feature-rich, cross-platform firmware development + HELLO environment for the UEFI and PI specifications from www.uefi.org. Core CI Build Status -- 2.19.1.3.g30247aa5d201
From 925e93af76af27526011ad33875fa26cc4cdc6c0 Mon Sep 17 00:00:00 2001 From: Laszlo Ersek <ler...@redhat.com> Date: Wed, 23 Jun 2021 16:19:51 +0200 Subject: [PATCH] world --- ReadMe.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/ReadMe.rst b/ReadMe.rst index 8f5db11281bf..cfc364db7406 100644 --- a/ReadMe.rst +++ b/ReadMe.rst @@ -4,6 +4,7 @@ EDK II Project A modern, feature-rich, cross-platform firmware development environment for the UEFI and PI specifications from www.uefi.org. + WORLD Core CI Build Status -------------------- -- 2.19.1.3.g30247aa5d201
