Re: development process failure [was: remove TPM related ppi from Depex for Fsp wrapper PEIM driver]

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

On 09/16/20 11:31, Yao, Jiewen wrote:
> Hi Laszlo
> Thanks. I agree 1, 2, 3. I take the blame. It is my fault.
> 
> For 4, it is out of my scope. I cannot find this by my eyes. Everything works well on my side.
> Can we improve patch checker to catch this in CI ?
> I don’t think I can find any Unicode in code or commit message easily.
> I prefer to let a tool to do that work.

Yes, we could perhaps enhance "BaseTools/Scripts/PatchCheck.py" to
require subjects to be 7-bit ASCII only. (And then some people would
disagree...)

I guess the idea is, unless it's a proper name being inserted in the
subject line, we should stick with 7-bit ASCII. For example, we should
reject U+FF1A (because U+003A is the right code point), but we should
still accept proper names in full UTF-8 (maybe not even restricted to
Latin script only)!

I don't know how this could be implemented in "PatchCheck.py"...

I guess what I would prefer is if contributors' input devices were
configured accordingly. When they press a key that promises to insert a
colon -- that is, when it *looks like* a colon --, then the symbol
should *become* a colon -- that is, U+003A, and not U+FF1A.

Thanks,
Laszlo


>> -----Original Message-----
>> From: Laszlo Ersek <lersek@redhat.com>
>> Sent: Wednesday, September 16, 2020 4:43 PM
>> To: Chiu, Chasel <chasel.chiu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
>> Cc: devel@edk2.groups.io; Zhang, Qi1 <qi1.zhang@intel.com>; Desimone,
>> Nathaniel L <nathaniel.l.desimone@intel.com>; Zeng, Star
>> <star.zeng@intel.com>; Wang, Jian J <jian.j.wang@intel.com>
>> Subject: development process failure [was: remove TPM related ppi from Depex
>> for Fsp wrapper PEIM driver]
>>
>> Jiewen, Chasel,
>>
>> On 09/15/20 08:21, Qi Zhang wrote:
>>> Some open board are TPM disabled. So the boot may hang because
>>>  these PPIs can't arrive. And gEdkiiTcgPpiGuid will be notified where
>>>   it is used. So we need to remove these PPIs from Depex for Fsp wrapper
>>>   PEI and PeiTpmMeasurementLib.
>>>
>>> Cc: Chasel Chiu <chasel.chiu@intel.com>
>>> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
>>> Cc: Star Zeng <star.zeng@intel.com>
>>> Cc: Jiewen Yao <jiewen.yao@intel.com>
>>> Cc: Jian J Wang <jian.j.wang@intel.com>
>>>
>>> Qi Zhang (2):
>>>   IntelFsp2WrapperPkg： remove gPeiTpmInitializationDonePpiGuid from
>>>     Depex
>>>   SecurityPkg/PeiTpmMeasurementLib: remove gEfiTpmDeviceSelectedGuid
>>>
>>>  IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf        | 3 +--
>>>  IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf        | 3 +--
>>>  .../Library/PeiTpmMeasurementLib/PeiTpmMeasurementLib.inf      | 3 +--
>>>  3 files changed, 3 insertions(+), 6 deletions(-)
>>>
>>
>> Please adopt a *much more* disciplined approach when merging patch series.
>>
>>
>> (1) When you merge a patch set, please report back on the list. Identify
>> both the pull request URL, and the commit reange.
>>
>> In this case, the pull request was
>>
>>   https://github.com/tianocore/edk2/pull/930
>>
>> and the commit range is a62fb4229d14..7bcb021a6d54.
>>
>>
>> (2) The associated Bugzilla:
>>
>>   https://bugzilla.tianocore.org/show_bug.cgi?id=2963
>>
>> has been completely neglected, by both submitter and maintainers.
>>
>> - The original BZ report is *absolute trash*.
>>
>> - No URL into the mailing list archive has been captured in the BZ,
>> about the posted series.
>>
>> - The BZ status is still CONFIRMED.
>>
>> - No mention of the pull request, or the resultant commit, range in the
>> BZ ticket.
>>
>>
>> (3) The github pull request at
>> <https://github.com/tianocore/edk2/pull/930> does contain *any*
>> indication of the bugzilla ticket, or the cover letter on the list.
>>
>> Basically we have random artifacts in three different places (Bugzilla,
>> github.com, mailing list), and nobody of the involved parties
>> (reviewers, maintainers, constributors) on this patch set have made
>> *any* effort to cross-reference them. We now have to hunt down
>> everything separately.
>>
>>
>> (4) Worst of all, the subject line of commit 414d7d11e6ea contains a
>> Unicode code point called FULLWIDTH COLON (U+FF1A) rather than a normal
>> colon (U+003A).
>>
>> Compare:
>>
>> - bad (current):           IntelFsp2WrapperPkg： remove [...]
>> - good (should have been): IntelFsp2WrapperPkg: remove [...]
>>
>> It makes absolutely no sense to use non-ASCII code points in subject
>> lines, for something as trivial as a colon.
>>
>>
>> I've been here for 8-9 years now and it's incredibly frustrating that I
>> *still* have to whine about basic stuff like this on a regular basis.
>>
>> I don't even know whom I should CC at Intel (management or otherwise) to
>> see an improvement in attitude here.
>>
>> I guess this community cannot be saved.
>>
>> Laszlo
>