Re: [EXTERNAL] [edk2-devel] [PATCH v4] UefiCpuPkg/MtrrLib/UnitTest: Add host based unit test

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

On 08/12/20 16:22, Ni, Ray wrote:
> Sorry I somehow pushed the https://github.com/niruiyu/edk2/commit/a1ca847e0a4f7f679d8c28c31d55c84f304d7a87 together with this patch to upstream master.

(That's probably commit 65904cdbb33c ("UefiCpuPkg/MtrrLibUnitTest:
Change to use static array for CI test", 2020-08-12) now.)

> The additional patch is to follow the suggestions in this thread, to make issues detected in CI reproduceable.
> 
> I created a pull request to trigger personal build to verify whether the additional change is good to pass CI. But it seems the Github automatically merged this with the original pull request (https://github.com/tianocore/edk2/pull/827) and the two changes then were pushed together because the push label was set.

If you pushed the new patch using the same branch that was the subject
of the original pull request (labeled "push"), then github must have
indeed taken the 2nd push as an "update" for the existent pull request.

> The push label was set 16 days ago and the patch cannot be merged due to the CI failure at that moment. But this time CI is happy so the patches are merged.

I don't like to leave PRs open for such a long time.

> I plan:
> 
>   1.  send the second patches for review

Sounds OK.

>   2.  revert the two patches

Why is it necessary to revert the first commit too? Was it not reviewed
either?

>   3.  add the Reviewed-by to the two patches and trigger the pull-request
> Is that ok?

Sure, in my opinion. I'd start with the revert though, as reviewing the
additional patch might take extra time.

Thanks
Laszlo