[edk2-devel] [PATCH] OvmfPkg/Bhyve: clean up TPM_ENABLE remnants

Michael D Kinney michael.d.kinney at intel.com
Tue Jun 22 15:38:07 UTC 2021


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?

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 <lersek at redhat.com>
> Sent: Tuesday, June 22, 2021 8:17 AM
> To: Kinney, Michael D <michael.d.kinney at intel.com>; devel at edk2.groups.io; spbrogan at outlook.com; ardb at kernel.org
> Cc: Peter Grehan <grehan at freebsd.org>; Ard Biesheuvel <ardb+tianocore at kernel.org>; Justen, Jordan L
> <jordan.l.justen at intel.com>; Sean Brogan <sean.brogan at microsoft.com>; Rebecca Cran <rebecca at 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 (#76848): https://edk2.groups.io/g/devel/message/76848
Mute This Topic: https://groups.io/mt/83497624/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-






More information about the edk2-devel-archive mailing list