[coreboot] Patch reviews

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Sun May 31 04:29:31 CEST 2009


Hi,

I noticed that many people are waiting for reviews/commits of their
patches. This can be a frustrating experience, so please let me explain
the situation.

Writing a patch is a _lot_ easier than reviewing it thoroughly. Even
one-liners can have a profound unwanted impact on code flow which is not
visible at a first glance.
Reviewing a patch is time-consuming. I usually spend at least twice the
time reviewing a patch compared to writing it myself.
Writing a patch is also much more pleasant than reviewing it. Writing a
patch is an act of creation whereas reviewing a patch is about finding
flaws.

That said, reviewing a patch thoroughly (that also includes reading the
code around it, the code it calls and the code it is called from) is a
great way to start with development.
If you want to get your hands dirty, I recommend looking for unapplied
patches and reviewing them. Don't be discouraged if someone else posts a
review in the meantime, your review is still valuable. If you find the
same things another reviewer has mentioned, still write about them (you
did find them as well, you deserve the credit). Write about everything
you checked even if it is OK. That shows others what you reviewed and
how you worked.

If you already know the codebase well, _please_ try to review unapplied
patches.

Patch reviewers are essential for the success of our project because:
- they keep buggy code out
- they keep bad design out
- they help patch submitters to improve their code
- they ensure steadily increasing code quality
- they show the patch submitter that someone cared about the patch.


If you have unapplied patches in your queue, please reply to them with a
small note ("ping" etc.) so reviewers can find them and look at them.


Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/





More information about the coreboot mailing list