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

[Ni, Ray]

> 
> 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".
> 
> I read the subject line and the commit message. Those make me anticipate
> some magic constant (related to 48) in the code. But that's not what I
> see in the code. I see new macros, new control flow, new variables, new
> indentation. The actual purpose of the patch (as documented in the
> commit message) is just a tiny fraction of the whole code change, and
> the commit message does not prepare the reader for it. *That* is what's
> wrong. Improving code wherever you go is great, but all that effort
> needs to be structured correctly, or at least justified in natural language.
> 
> Patches exist primarily for humans to read, and secondarily for
> computers to execute. If we don't believe in that, then edk2 will never
> become a true open source, community project. (In my opinion anyway.)
> 

I admit my patch assumes the reviewers should be very familiar to how CPUID "protocol" works.
In fact, there are two kinds of reviewers at least:
1. domain reviewers
2. consumers as reviewers

I didn't consider the second kind of reviewers.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#75410): https://edk2.groups.io/g/devel/message/75410
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]
-=-=-=-=-=-=-=-=-=-=-=-