Re: [virtio-comment] RE: [PATCH v10 06/10] mmio: document ADMIN_VQ as reserved

[Cornelia Huck <cohuck@redhat.com>]

On Tue, Mar 07 2023, Parav Pandit <parav@nvidia.com> wrote:

>> From: Michael S. Tsirkin <mst@redhat.com>
>> Sent: Friday, March 3, 2023 3:34 AM
>> 
>> On Thu, Mar 02, 2023 at 06:40:55PM +0000, Parav Pandit wrote:
>> > Did you miss reviewed-by from [1] or this is an old series reposted?
>> > [1]
>> > https://lists.oasis-open.org/archives/virtio-dev/202302/msg00242.html
>> 
>> As a general rule, we don't strictly need to track reviewed by since there's a
>> ballot (and presumably people review before voting).
>> 
>> People also tack on Signed-off-by: (and I do it too) but as long as we don't
>> document what it means it's kind of vague, and the process of subscribing to
>> the mailing list is a kind of replacement.
>> 
>> If you think everyone needs to follow practices like netdev does, we really need
>> something written up, and agree on it.
>> 
>> E.g.  I work on the linux kernel too, so I can copy practices from there, but even
>> linux isn't uniform.
>> 
>> And I wonder whether it's worth it - it definitely makes contributing to Linux
>> harder, and even within Linux it pushes contributors away. 
> The number of virtio spec contributors are in order of magnitude less than Linux kernel or just the Linux netdev.
> Patch split up reduces lot of time author and reviewers.
> For example, interrupt moderation at v10 can be very easily split up where prep patch can have RB tag and v11 doesn't need reviewers and author's time.
> Your work here with 10 patches is yet another good example of it.
> I remember Max and I started with 6 patches... and current 10 are more useful way to split them.

I'd argue that splitting changes in a way that makes it easy for
reviewers is a good thing for any project (although practices on many
forges seem to go into another direction...)

>
>> At least for Linux
>> tracking history in a precise way is extremely important since it's helpful with
>> stability. Spec is very different.
>> 
>> Until we have a good contribution documentation I think we should not ask
>> people to follow a pseudo Linux work flow with requests like "please split this
>> patchset up" or "track changes across patch versions"
>> simply because there's no good docs to teach people what exactly is the best
>> practice.
>
> Yes, I wanted to update the contributing document. It is in my to-do list.
> But given the small crowd of contributors right now, almost everyone is familiar with the RB, ack tag.
> At moment it has two main reasons.
>
> 1. Acknowledge the work and efforts that go in the review

I agree, and I try to include tags when applying.

> 2. When in question, reach out later to the spec author or reviewers to know about context/design etc.
> 3. Same reviewer doesn't need to go through the patch which already has RB tag of him/her.

I tend to use R-b in that way as well, but it relies on the patch author
not doing substantial changes between versions...

I guess the usage of R-b et al in the virtio spec stems from the fact
that many contributors are also Linux and/or QEMU contributors  -- not
sure if it makes sense to enshrine their usage, but

4. We can get an R-b from a non-TC member who is an expert on the topic
(and follows standard Linux/QEMU/... practices.)

In the end, how the TC members are voting is the only thing that matters
for inclusion.