Jump to: navigation, search

Difference between revisions of "Reviewer guide (Zaqar)"

m (Use Prefixes)
m (Use Prefixes)
Line 31: Line 31:
 
! Prefix !! What the reviewer is saying !! Blocker?
 
! Prefix !! What the reviewer is saying !! Blocker?
 
|-
 
|-
| TEST || Missing a test for this feature, code branch, specific data input, etc. || Yes
+
| TEST || I think you are missing a test for this feature, code branch, specific data input, etc. || Yes
 
|-
 
|-
 
| BUG || I don't think this code does what it was intended to do, or I think there is a general design flaw here that we need to discuss. || Yes
 
| BUG || I don't think this code does what it was intended to do, or I think there is a general design flaw here that we need to discuss. || Yes

Revision as of 22:14, 18 June 2014

Overview

Our program follows the usual OpenStack review process, albeit with some important additions (see below). See also: Your First Review (Marconi).

Be Professional

The PTL, with the support of core reviewers, is ultimately responsible for holding contributors accountable for creating a positive, constructive, and productive culture. Inappropriate behavior will not be tolerated.

Do This:

  • Act professionally
  • Treat others as friends and family
  • Seek first to understand
  • Be honest, transparent, and constructive
  • Use clear, concise language
  • Use prefixes to clarify the tone and intent of your comments

Don't Do This:

  • Be a jerk
  • Use indecent, profane, or degrading language of any kind
  • Hold a patch hostage for an ulterior motive, political or otherwise.
  • Abuse the review system to discuss big issues that would be better hashed out on the mailing list, in IRC, or through a G+ hangout.

Use Prefixes

We encourage the use of prefixes to clarify the tone and intent of your review comments. This is one way we try to mitigate misunderstandings that can lead to bad designs, bad code, and bad blood.

Prefix What the reviewer is saying Blocker?
TEST I think you are missing a test for this feature, code branch, specific data input, etc. Yes
BUG I don't think this code does what it was intended to do, or I think there is a general design flaw here that we need to discuss. Yes
DSQ I'd like to discuss this in IRC or on the mailing list. Yes
PERF I have a concern that this won't be fast enough or won't scale. Maybe
Q I don't understand something. Can you clarify? Maybe
NIT This is a nitpick that I can live with if we want to merge without addressing it. No
IMO I'm chiming in with my opinion in response to someone else's comment, or I just wanted to share an observation. Maybe
FYI I just wanted to share some useful information. No