linuxppc patchwork queue

[Michael Ellerman]

On Tue, 2015-11-24 at 08:56 +1100, Daniel Axtens wrote:
> > The main thing you could do, and anyone could do (!), is just review some
> > patches. Even if you don't know the area of code that well, you can usually do
> > a basic review.
> > 
> > eg. Just the basic stuff:
> >  - Is the subject correctly formatted and makes sense.
> >  - Is the change log well written and describes the change well.
> >  - Does the patch do one logical change.
> >  - Are there any obviously bad coding style things.
> >  - Can you follow the basics of the change based on the change log and some
> >    reading of the code?
> 
> Michael gave me the same advice earlier this year, and I found it really
> helpful. As a result, I've been trying to do reviews.
> 
> I am by no means an expert, but here's what I found starting out:
> 
>  - Reviewing is a learned skill - you don't just start doing kernel dev
>    and get this magical sense of how to do reviews. This means it's OK
>    to start small, and that no-one expects your early reviews to be
>    amazing.

Yes!

>  - Reviewing gets much easier when you just jump in and start doing it!
> 
>  - A review doesn't have to focus on getting to a reviewed-by
>    tag. Asking good questions is equally if not more valuable.

Absolutely.

In fact I'm usually a bit sceptical of reviews that are only a Reviewed-by tag,
because that gives me no evidence that the reviewer actually reviewed it.

I'd much rather some questions and/or comments. Even if the patch is perfect,
there's almost always something that you can comment on. Even if it's just
observations like "this logic confused me until I realised X which makes it
work".

>  - It was easier to start with the 'little picture', or what Michael
>    calls the 'basic stuff'. Is it good C? Does the commit message make
>    sense? Is the spelling right? Does it do weird things without
>    explaining them? When you're starting it's OK to leave the 'big
>    picture'/architectural/design reviews to other people. I definitely
>    still do!

Yes exactly. Often just starting to review those things will give you enough
insight into the patch to start understanding it a bit more, or at the very
least you prompt the author into explaining what they're doing better, which is
almost always helpful.

cheers