Re: [edk2-devel] [Patch v2 1/2] SecurityPkg: Replace EFI_D_* with DEBUG_*

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

Hi Mike,

On 10/22/19 19:37, Michael D Kinney wrote:
> Update all DEBUG() macros in the SecurityPkg to use
> DEBUG_ instead of EFI_D_.  This is required to pass
> PatchCheck.py checks.

[...]

>  45 files changed, 410 insertions(+), 410 deletions(-)

(

If the SecurityPkg maintainers are happy with this patch, then it's not
my place to complain.

I'd just like to point out that I'd object to such a patch for OvmfPkg.
Such sweeping conversions are difficult to review (they are also
difficult to implement -- I think mass search&replace is not too safe
without human review).

New code should not add EFI_D_* usage, of course.

I'd expect PatchCheck.py to complain about EFI_D_* only on lines that
are added by a patch, not on lines being removed, or present in the
context. Is that not the case?

... Hm, looking at patch#2, it seems that some spelling errors affect
debug messages. Therefore, some of the typo fixes do turn EFI_D_* macros
into new lines. Given that there is a huge number of typo fixes (205
lines, apparently), I guess it makes sense to separate out the EFI_D_*
conversion. It's up to the package owners whether they prefer reviewing
- 410 lines of EFI_D_* massaging, plus 205 lines of typo fixes,
- or 205 lines of { EFI_D_* conversion, plus typo fix }.

For OvmfPkg, my choice would likely be (assuming such a large diffstat):
- fix EFI_D_*, one patch per module, and only on lines affected by typos,
- fix typos, one patch per module.

I could suspend and resume a review like that more easily.

)

Thanks
Laszlo