From: Daniel Axtens <dja@axtens.net>
To: Michael Ellerman <mpe@ellerman.id.au>,
Denis Kirjanov <kda@linux-powerpc.org>
Cc: linuxppc-dev <linuxppc-dev@lists.ozlabs.org>
Subject: Re: linuxppc patchwork queue
Date: Tue, 24 Nov 2015 08:56:54 +1100 [thread overview]
Message-ID: <87r3jgv0pl.fsf@gamma.ozlabs.ibm.com> (raw)
In-Reply-To: <1448273552.3452.7.camel@ellerman.id.au>
[-- Attachment #1: Type: text/plain, Size: 1865 bytes --]
> 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.
- 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.
- 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!
HTH.
Daniel
>
> Another thing I seem to spend a lot of time on is asking people if their patch
> should go to stable or not, and if so what versions. So everyone could help out
> there.
>
> cheers
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 859 bytes --]
next prev parent reply other threads:[~2015-11-23 21:57 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-11-20 14:48 linuxppc patchwork queue Denis Kirjanov
2015-11-23 0:15 ` Michael Ellerman
2015-11-23 9:45 ` Denis Kirjanov
2015-11-23 10:12 ` Michael Ellerman
2015-11-23 21:56 ` Daniel Axtens [this message]
2015-11-26 6:49 ` Michael Ellerman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87r3jgv0pl.fsf@gamma.ozlabs.ibm.com \
--to=dja@axtens.net \
--cc=kda@linux-powerpc.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=mpe@ellerman.id.au \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).