Re: [PATCH net-next] docs: try to encourage (netdev?) reviewers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Matt,

On Tue, Oct 10, 2023 at 5:19 PM Matthieu Baerts <matttbe@xxxxxxxxxx> wrote:
> On 10/10/2023 00:56, Jakub Kicinski wrote:
> > Add a section to netdev maintainer doc encouraging reviewers
> > to chime in on the mailing list.
> >
> > The questions about "when is it okay to share feedback"
> > keep coming up (most recently at netconf) and the answer
> > is "pretty much always".
> >
> > Extend the section of 7.AdvancedTopics.rst which deals
> > with reviews a little bit to add stuff we had been recommending
> > locally.
>
> Good idea to encourage everybody to review, even the less experimented
> ones. That might push me to send more reviews, even when I don't know
> well the area that is being modified, thanks! :)
>
> (...)
>
> > diff --git a/Documentation/process/7.AdvancedTopics.rst b/Documentation/process/7.AdvancedTopics.rst
> > index bf7cbfb4caa5..415749feed17 100644
> > --- a/Documentation/process/7.AdvancedTopics.rst
> > +++ b/Documentation/process/7.AdvancedTopics.rst
> > @@ -146,6 +146,7 @@ pull.  The git request-pull command can be helpful in this regard; it will
> >  format the request as other developers expect, and will also check to be
> >  sure that you have remembered to push those changes to the public server.
> >
> > +.. _development_advancedtopics_reviews:
> >
> >  Reviewing patches
> >  -----------------
> > @@ -167,6 +168,12 @@ comments as questions rather than criticisms.  Asking "how does the lock
> >  get released in this path?" will always work better than stating "the
> >  locking here is wrong."
>
> The paragraph just above ("it is OK to question the code") is very nice!
> When I'm cced on some patches modifying some code I'm not familiar with
> and there are some parts that look "strange" to me, I sometimes feel
> like I only have two possibilities: either I spend quite some time
> understanding that part or I give up if I don't have such time. I often
> feel like I cannot say "I don't know well this part, but this looks
> strange to me: are you sure it is OK to do that in such conditions?",
> especially when the audience is large and/or the author of the patch is
> an experienced developer.

Yes you can (even experienced developers can make mistakes ;-)!

If it is not obvious that something is safe, it is better to point it
out, so the submitter (or someone else) can give it a (second) thought.
In case it is safe, and you didn't miss the ball completely, it probably
warrants a comment in the code, or an improved patch description.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds



[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux