Re: [edk2-devel] [PATCH 1/2] OvmfPkg: start using the ECC plugin exception list

["Laszlo Ersek" <lersek@redhat.com>]

On 12/08/20 19:45, Sean Brogan wrote:
> Laszlo,
> 
> Trying to understand this.
> 
> 
> On 12/7/2020 5:56 PM, Laszlo Ersek wrote:
>> ... I would still much prefer of course if that patch (= the exception
>> to GuidCheck) could simply be included in James's series.
> 
> 
> The PR based CI runs on the entire series.  It does not run on
> individual commits and thus you can add this to the series and in fact i
> would suggest it get added to the series.  Any changes in the series
> will take effect when running the CI.

That sounds great, thank you. I must have misunderstood something in the
past. I distinctly remember coming away from a discussion somewhere with
the lesson that CI tweaking had to be done in a separate PR.

Perhaps I mistook an explanation. I guess my mental image was that the
CI run started on "master", grabbing its config from the files in
"master", and then checking out or otherwise applying the patches for
the subject series. If CI indeed *launches* while standing at the HEAD
of the topic branch, then that's great.

(I know from my local CI experimentation that it just runs on whatever
commit I have checked out, modulo uncommitted changes per
<https://bugzilla.tianocore.org/show_bug.cgi?id=2986> -- but I didn't
know if the CI environment inside github / azure was the same.)

> 
> This is great for how Project Mu uses "PR gates" because we squash merge
> but in Edk2 with a patch series this can mean that commits in the middle
> can break things.  It is on developer and reviewer to catch those types
> of things.
> 
> For this case why can't this change be part of the commit that
> introduces the guid/global?

I agree that it should be.

> 
> Or if that is undesirable you should be able to add the ignore in a
> commit prior to introduction and then you would never have a break.
> Either way there is no reason this isn't part of a single series.

Sounds great, thank you. I actually prefer it to be part of the same commit.

Here's what I'm proposing / requesting:

(1) Sean, could you please (pretty please :) ) submit the second patch

  Disable EccCheck for OvmfPkg CI

from your demo PR at

  https://edk2.groups.io/g/devel/message/68541
  https://github.com/tianocore/edk2/pull/1201

stand-alone to the list, so that Ard or myself can ACK it and merge it
separately?

(2) Subsequently, I'm going to take the 2nd patch of the present series,
to which Ard's R-b applies as well:

  https://edk2.groups.io/g/devel/message/68451

and I'll *squash it* into James's

  [PATCH v3 3/6] OvmfPkg/AmdSev: add Grub Firmware Volume Package

And then I'll merge that v3 series.

Thank you!
Laszlo