Re: [edk2-devel] [PATCH v2 5/5] OvmfPkg: link SM3 support into Tcg2Pei and Tcg2Dxe

["Imran Desai" <imran.desai@intel.com>]

Hello Lersek/ Leif,

Like Lersek hinted, this in fact was an inadvertent issue with the git configuration. 
I suspect the suppresscc  in my configuration may have been the source of all confusion. My apologies.

I will send in a new version v5 of the full series of the patches.

I appreciate all the help and comments. Thanks.

PS:

Being used to GitHub pull request methods where in all reviews and conversation happen on the same page i was a little thrown by this process to be honest. In any case as a first time contributor this was a hard way to find out. Perhaps it could be helpful to update the https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process. Particularly the line "git send-email *.patch" could use further commentary regarding CC list.

Thanks and Regards,

Imran Desai
________________________________________
From: Leif Lindholm [leif.lindholm@linaro.org]
Sent: Wednesday, July 03, 2019 2:18 PM
To: devel@edk2.groups.io; lersek@redhat.com
Cc: Desai, Imran; Wang, Jian J; Justen, Jordan L; Ard Biesheuvel; Marc-André Lureau; Stefan Berger; Cetola, Stephano; Kinney, Michael D; Andrew Fish
Subject: Re: [edk2-devel] [PATCH v2 5/5] OvmfPkg: link SM3 support into Tcg2Pei and Tcg2Dxe

On Wed, Jul 03, 2019 at 10:37:40PM +0200, Laszlo Ersek wrote:
> On 05/28/19 22:40, Imran Desai wrote:
> >
> > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1781
> >
> > EDK2 Support for SM3 digest algorithm is needed to enable TPM with SM3 PCR
> > banks. This digest algorithm is part of the China Crypto algorithm suite.
> > This integration has dependency on the openssl_1_1_1b integration into
> > edk2.
> > This patch links SM3 support into Tcg2Pei and Tcg2Dxe.
> >
> >
> > Signed-off-by: Imran Desai <imran.desai@intel.com>
> > Cc: Jordan Justen <jordan.l.justen@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Cc: Stefan Berger <stefanb@linux.ibm.com>
> > ---
> >  OvmfPkg/OvmfPkgIa32.dsc    | 2 ++
> >  OvmfPkg/OvmfPkgIa32X64.dsc | 2 ++
> >  OvmfPkg/OvmfPkgX64.dsc     | 2 ++
> >  3 files changed, 6 insertions(+)
>
> Wow, what just happened here?
>
> I'm noticing now that this patch has been pushed to the master branch as
> commit a7c7d21ffa9a.
>
> However, *NONE* of the OvmfPkg co-maintainers or reviewers have reviewed
> this patch!

Nor has Jian, or Jiewen, on-list - there is only the tags in the commit.
Other patches in the set have R-b: from Jiewen in the commit, but never
on the list.

Not that it would matter. Patches do not go in without package
maintainer review. Unless there is a *really* good reason, at which
point I expect at least one steward to Ack/R-b:.

> The commit message includes "Cc:" lines, but that's a lie. Probably not
> an intentional lie, but a lie nonetheless. These patches have *never*
> been delivered to my inbox, and if I look at the address list on the
> message instance that was reflected by the mailing list, that address
> list confirms the same. I'm pretty sure Imran's git configuration has a
> bug related to CC's. (I've extended the address list now, manually.)
>
> Jian: please revert this patch immediately, stating, as reason, that the
> patch review process was not honored. I'm sorry but I cannot let this
> slide -- if you look at commit a7c7d21ffa9a now, it suggests that the
> OvmfPkg maintainers / reviewers were CC'd (they weren't), but they
> ignored the patch (they didn't -- they couldn't see it), and that
> another maintainer pushed the patch just the same (which is factual, but
> *wrong*).

Based on the included Reviewed-by tags from Jiewen that never appeared
on the list, this entire series need to be reverted.

1/5 then needs Reviewed-by: from an MdePkg maintainer, after which
2-4/5 can be pushed again if the commit messages are updated to
contain only the Reviewed-by: and Cc: that have actually happened
on-list.

/
    Leif