Message-ID: <1da16601-dc03-4b65-252f-3925f2a83705@kernel.org>
Date:   Mon, 17 Jul 2023 09:44:35 +0200
From:   Krzysztof Kozlowski <krzk@...nel.org>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     corbet@....net, workflows@...r.kernel.org,
        linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
        gregkh@...uxfoundation.org
Subject: Re: [PATCH docs] docs: maintainer: document expectations of small
 time maintainers

On 14/07/2023 19:10, Jakub Kicinski wrote:
>>> +
>>> +Maintainers must review *all* patches touching exclusively their drivers,  
>>
>> I don't agree with this as a small driver maintainer. Several subsystem
>> maintainers take the patches much faster than I am able to check the
>> inbox. I can provide names if you need some proves. With such criteria I
>> should be removed from maintainers, because I am not able to review
>> within 24h.
>>
>> Either give reasonable time, like two weeks, or don't require driver
>> maintainers to be 24/7 for subystem maintainer disposal. This is very
>> unfair rule.
> 
> I think your concern is more about the timeline than what's quoted here,
> so I rephrased that:

My concerns are for both timeline and for wording which makes it
obligatory. I think we should not have stale maintainers in MAINTAINERS
file, thus if someone repeatedly does not match criteria, should be
dropped and moved to CREDITS. However I felt here your wording quite
strong, thus I would assume we will start dropping a lot, a lot of
driver maintainers. I am not sure if we really want it, because from
time to time, such maintainer might be actually active and helpful.

> 
> -The exact expectations on the review time will vary by subsystem
> -from 1 day (e.g. networking) to a week in smaller subsystems.
> 
> +The exact expectations on the response time will vary by subsystem.
> +The patch review SLA the subsystem had set for itself can sometimes
> +be found in the subsystem documentation. Failing that as a rule of thumb
> +reviewers should try to respond quicker than what is the usual patch
> +review delay of the subsystem maintainer. The resulting expectations
> +may range from two working days for fast-paced subsystems to two weeks
> +in slower moving parts of the kernel.

Sounds good. Thank you.

> 
> 
> To the point of reviewing "all" patches, I want to keep this. When 
> I ping vendors they often reply with "oh I didn't know I'm supposed
> to respond, the change looks good". People confuse the review process
> with a veto process, if they don't want to outright reject the change
> they stay quiet :|

OK, I understand. That's the good point.

> 
>>> +no matter how trivial. If the patch is a tree wide change and modifies
>>> +multiple drivers - whether to provide a review is left to the maintainer.
>>> +
>>> +There should be multiple maintainers for any piece of code, an ``Acked-by``
>>> +or ``Reviewed-by`` tag (or review comments) from a single maintainer is
>>> +enough to satisfy this requirement.
>>> +
>>> +If review process or validation for a particular change will take longer
>>> +than the expected review timeline for the subsystem, maintainer should
>>> +reply to the submission indicating that the work is being done, and when
>>> +to expect full results.
>>> +
>>> +Refactoring and core changes
>>> +----------------------------
>>> +
>>> +Occasionally core code needs to be changed to improve the maintainability
>>> +of the kernel as a whole. Maintainers are expected to be present and
>>> +help guide and test changes to their code to fit the new infrastructure.
>>> +
>>> +Bug reports
>>> +-----------
>>> +
>>> +Maintainers must respond to and address bug reports. The bug reports  
>>
>> This is even more unreasonable than previous 1 day review. I don't have
>> capabilities to address bug reports for numerous drivers I am
>> maintaining. I don't have hardware, I don't have time, no one pays me
>> for it. I still need some life outside of working hours, so expecting
>> both reviews in 1 day and addressing bugs is way too much.
>>
>>> +range from users reporting real life crashes, thru errors discovered
>>> +in fuzzing to reports of issues with the code found by static analysis
>>> +tools and new compiler warnings.
>>> +
>>> +Volunteer maintainers are only required to address bugs and regressions.  
>>
>> "Only required"? That's not "only" but a lot.

Thanks.

> 
> I was trying to soften the paragraph for volunteers let me try to
> soften it.. harder?
> 
>>> +It is understood that due to lack of access to documentation and
>>> +implementation details they may not be able to solve all problems.  
>>
>> So how do I address? Say "Oh, that's bad"?
> 
> How about:
> 
>   Bug reports
>   -----------
> 
>   Maintainers must respond to bug reports of reasonable quality. The bug reports
>   range from users reporting real life crashes, thru errors discovered
>   in fuzzing to reports of issues with the code found by static analysis
>   tools and new compiler warnings.
> 
>   It is understood that the hands of volunteer maintainers can often be tied
>   by the lack of access to documentation, implementation details, hardware
>   platforms, etc.
> 
> 
> I don't know how to phrase it better :( Obviously maintainers are
> expected to look at bug reports. At the same time we all know the
> feeling of being a maintainer of some crappy HW which sometimes 
> doesn't work and all we can do is say "thoughts and prayers". 

Yes, sounds better.

> 
> IDK. 
> 
> The doc would be incomplete without mentioning that bug reports are
> part of maintainers' life :(
> 
>> Jakub, with both of your criteria - reviewing and addressing - I should
>> be dropped from all the driver maintainership. If this document passes,
>> I will do it - drop myself - because:
>> 1. No one pays me for it,
>> 2. I barely have hardware,
>> 3. I want to live a life and I am already working much more than 8h per day.
> 
> It's really hard to codify the rules. I hope we can start somewhere
> and chisel at the rules if/as we start getting feedback/complaints.
> 
> I can give you examples of bad vendor behavior or people who stopped
> participating 10 years ago yet they still figure in MAINTAINERS all day.

Yep, I understand and I was cleaning such entries as well... :)

> Next time I see a rando manager added as a maintainer I want to be able
> to point them at a document. If the document is too "soft" they will
> just wave it off :(

Best regards,
Krzysztof

Powered by blists - more mailing lists