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

[Michael Brown]

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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#75419): https://edk2.groups.io/g/devel/message/75419
Mute This Topic: https://groups.io/mt/82765279/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-