Re: [edk2-devel] [Patch 1/1] Maintainers.txt: Update based on active community members

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

On 10/30/23 03:40, Yao, Jiewen wrote:
> Thanks Mike. I am reading the WIKI page.
> 
> 
>> and/or provides testing or regression testing for the Package (or some modules thereof), in certain platforms and environments.
> 
> [Jiewen] Are we expecting Reviewer to provide testing or regression testing for the package?
> Is that what the reviewer *commits* to do?
> For example, Maintainer may ask the reviewer to do some testing, right?

It depends on the reviewer's individual commitment.

First of all, the burden of testing / regression-testing, to a
reasonable extent [1], is on the submitter.

[1] In some cases, the submitter cannot test the code they modify in all
possible environments / circumstances. In such cases, the submitter
should test the change code wherever they can, as widely as they can,
and be upfront (in the commit message) about lacking coverage in other
environments they might be aware of. It is then fine for the maintainer
(or even reviewer) to ask others for further / wider testing, but trying
to saddle someone with that testing as an *obligation* will never fly.
That would only alienate people from contributing.

This was the primary reason for splitting Xen code as sharply as
possible from non-Xen code in both ArmVirtPkg and OvmfPkg. Xen and
QEMU/KVM are so different environments, with so distinct audiences, that
keeping code common was *worse* than duplicating and customizing code.
We could never *sufficiently* regression-test our changes for each
other. It was best to separate those areas of interest. Demanding that I
regression-test on Xen, or that Anthony or Julien regression-test on
QEMU/KVM, would have lead nowhere.

Second, the level of commitment varies. A reviewer may have a fleeting
interest in a module (just want to be in the loop), or else they may be
completely invested in it, and they might actually prefer being a
maintainer.

For example, I had seen many bad regressions in OVMF due to UefiCpuPkg
patches, thus, even thouogh I absolutely didn't welcome the additional
responsibility, I asked to be added as a Reviewer for UefiCpuPkg. With
that, I wanted to formalize my request to be CC'd on all UefiCpuPkg
patches, but I also committed to regression testing, and maybe even
reviewing, those patches. It worked out quite well, but of course I was
still selective in what I would review and test. If I could immediately
determine that the patch modified code in UefiCpuPkg that never ran on
(or wasn't even built into) OVMF, I would clearly state on the list that
I'd not review or test the patch, i.e., that nobody should wait for me.

> 
> 
>> Reviewer is responsible for timely responses on emails addressed to them (preferably less than calendar week).
> 
> [Jiewen]
> Is that what the reviewer *commits* to do?

What I think we can expect a reviewer to *commit* to is to say
*something* reasonably quickly. The whole idea is to support others in
making a *decision*, in making progress. So the "something" the reviewer
says may well be:

- "this does not apply to the area I have expertise or interest in, so
please proceed with this patch without my feedback (testing or review or
opinion etc)"

- "I don't have time for this right now, so please go ahead; if it
breaks, we'll figure it out later" (the maintainer need not accept this,
and might want to block the patch independently for a bit longer, until
someone else provides the desired review or testing, but the reviewer is
still totally OK to say this)

- "please give me a few more days to review this set".

> For example, Maintainer may ask the reviewer to provide feedback, right?

IMO, the maintainer is welcome to request feedback, of course; that's
presumably why the reviewer wanted to be listed in Maintainers.txt in
the first place.

Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110305): https://edk2.groups.io/g/devel/message/110305
Mute This Topic: https://groups.io/mt/102245264/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-