Code Review: Difference between revisions

From MozillaWiki
Jump to navigation Jump to search
Line 14: Line 14:
== Bugzilla Improvements ==
== Bugzilla Improvements ==
* In addition to r+/r- add eta flag to specify when the patch will get reviewed. Bugzilla should send reminder emails based on that flag.
* In addition to r+/r- add eta flag to specify when the patch will get reviewed. Bugzilla should send reminder emails based on that flag.
* Provide information about reviewers when asking for r?
** Add a notion of being away to bz-accounts.
** Display number of outstanding reviews

Revision as of 21:55, 10 July 2013

Reviewer Guidelines

  1. Prioritize. Reviewing code is more valuable than writing code as it results in higher overall project activity. If you find you can't write code anymore due to prioritizing reviews over coding, grow more reviewers.
  2. Communicate. If you are an active contributor, you should not leave r? patches sitting in your queue without feedback. "I will review this next week because I'm (busy reviewing ___ this week|away at conference). If you think a patch is lower priority than your other work communicate that.
  3. Silence is not an option. If you think saying nothing is better than admitting than you wont get to the patch for a while(Holding back bad code is a feature, not a bug, do it politely), that's passive aggressiveness (https://en.wikipedia.org/wiki/Passive-aggressive_behavior). This is not a good way to build a happy coding community.

Patch-author Guidelines

There are some things that make it easier to get a good, prompt review, and to improve the quality of the patch you submit. Help your reviewers review your patches quickly by making patches easy to review:

  • small patches, functionally divided;
  • don't request review from people who are labeled as being away in Bugzilla and expect fast turnaround

Bugzilla Improvements

  • In addition to r+/r- add eta flag to specify when the patch will get reviewed. Bugzilla should send reminder emails based on that flag.
  • Provide information about reviewers when asking for r?
    • Add a notion of being away to bz-accounts.
    • Display number of outstanding reviews