Re: [PATCH 0/6] OvmfPkg/ResetSystemLib: clean up, refactor, fix

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 4/20/20 11:48 AM, Laszlo Ersek wrote:
> On 04/17/20 18:19, Philippe Mathieu-Daudé wrote:
>> On 4/17/20 5:59 PM, Ard Biesheuvel wrote:
>>> On 4/17/20 5:37 PM, Laszlo Ersek wrote:
>>>> Ref:    https://bugzilla.tianocore.org/show_bug.cgi?id=2675
>>>> Repo:   https://pagure.io/lersek/edk2.git
>>>> Branch: rsl_cleanup
>>>>
>>>> Rebecca's
>>>>
>>>>     [PATCH 02/13] OvmfPkg: support powering off bhyve guests
>>>>
>>>> at
>>>>
>>>>     https://edk2.groups.io/g/devel/message/57450
>>>>    
>>>> http://mid.mail-archive.com/e4e9b29189b83076e1d1a0b9c989938f5226cab6.1586991816.git.rebecca@bsdio.com
>>>>
>>>>
>>>> made me realize OvmfPkg/ResetSystemLib should be refreshed. This will
>>>> let us add a simple bhyve-specific instance (later), and also allows us
>>>> to fix a long time dormant bug (now).
>>>>
>>>> Cc: Anthony Perard <anthony.perard@citrix.com>
>>>> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
>>>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>>>> Cc: Julien Grall <julien@xen.org>
>>>> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>> Cc: Rebecca Cran <rebecca@bsdio.com>
>>>>
>>>> Thanks,
>>>> Laszlo
>>>>
>>>> Laszlo Ersek (6):
>>>>     OvmfPkg/ResetSystemLib: wrap long lines
>>>>     OvmfPkg/ResetSystemLib: clean up library dependencies
>>>>     OvmfPkg/ResetSystemLib: improve coding style in ResetSystem()
>>>>     OvmfPkg/ResetSystemLib: factor out ResetShutdown()
>>>>     OvmfPkg/ResetSystemLib: rename to BaseResetSystemLib
>>>>     OvmfPkg/ResetSystemLib: introduce the DxeResetSystemLib instance
>>>>
>>>
>>> For the series,
>>>
>>> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
>>>
>>> One nit: putting a diff block inside the commit log [6/6] doesn't help
>>> legibility a lot, and the issue of not being able to access memory
>>> that is not mapped for runtime is so basic that it doesn't require
>>> that level of detail to describe a reproducer and the Linux kernel log
>>> output when the issue is triggered.
>>
>> Personally I find the kernel log relevant, it will help to understand th
>> e patch when looking at it in >5years from now.
> 
> Is it acceptable to both of you (Phil and Ard) if I remove the third
> note from the 6/6 commit message, but paste it into a new comment on
> <https://bugzilla.tianocore.org/show_bug.cgi?id=2675>?

Certainly.

Personal note: I usually try to find answers to my questions on the 
mailing list archives, usually they are already answered (even often 
with many details!). I noticed with other (smaller) projects that moved 
to GitLab that 'googling' similarly is not very practical, eventually 
after learning something that looks like a mix of SQL and JSON I could 
find some information in failed merge request comments... Nothing as 
easy as grepping my local MAILBOX (which, by the way, works offline 
too). Having that in mind (that mailing list archives are a thing from 
the previous century and are going to disappear) I prefer to keep all 
the information in the commit message. There is always a trade off.

I'm not sure the Tianocore Bugzilla will make sense once EDK2 move to a 
git forge, it seems duplicating efforts.

My 2 cents.

> Thanks!
> Laszlo
>