Re: [edk2-devel] [PATCH] UefiCpuPkg/PiSmmCpu: Remove hardcode 48 address size limitation

["Michael Brown" <mcb30@ipxe.org>]

On 18/05/2021 19:42, Laszlo Ersek wrote:
> On 05/18/21 09:51, Ni, Ray wrote:
>> Thanks for explaining why you don't think it's a good patch. I thought anytime changing a code,
>> I should try to make it better, functional better, looks better.
> 
> My only point was that separate concerns should be implemented in
> separate patches, or at least (if they are really difficult, or
> overkill, to isolate) that they should be documented.
> 
> Please try to think with your reviewers' mindsets in mind, when
> preparing a patch (commit message and code both). The question the patch
> author has to ask themselves is not only "how do I implement this", but
> also "how do I explain this to my reviewers".

(Apologies in advance for intruding.)

Ray: I think you may be neglecting one half of the problem.

When making a code change, there are (at least) two requirements:

1. The new version of the code must make sense.  You are definitely 
achieving this: as you say, "make it better, functional better, looks 
better".  This is good.

2. The *change* between the old and new versions of the code (i.e. the 
patch and accompanying commit message) must also make sense.  This is 
the requirement that I think Laszlo would like you to meet.

It's not viable for anyone to meaningfully review code unless both of 
these requirements are met.

Thanks,

Michael