Reviewer guide (Zaqar)

Overview
Our program follows the usual OpenStack review process, albeit with some important additions (see below). See also: Your first review on Zaqar.

Be Professional
The PTL, with the support of the core reviewers, is ultimately responsible for holding contributors accountable for creating a positive, constructive, and productive culture. Inappropriate behavior will not be tolerated. (Why is this important?)

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:

 * 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.
 * Engage in bullying behaviors, including but not limited to:
 * Belittling others' opinions
 * Persistent teasing or sarcasm
 * Insulting, threatening, or yelling at someone
 * Accusing someone of being incompetent
 * Setting someone up to fail
 * Humiliating someone
 * Isolating someone from others
 * Withholding information to gain an advantage
 * Falsely accusing someone of errors
 * Sabotaging someone's work

Reviewing Docs
When possible, enlist the help of a professional technical writer to help review each doc patch. All reviewers should familiarize themselves with How to Review a Documentation Patch. When reviewing user guide patches, please run them through Maven and proof the resulting docs before giving your +1 or +2.

Reviewing Code
When reviewing code patches, use your best judgment and seek to provide constructive feedback to the author. Compliment them on things they have done well, and highlight possible improvements. Also, dedicate as much time as necessary in order to provide a careful analysis of the code. Don't assume that someone else will catch any issues you yourself miss; in other words, pretend you are the only person reviewing a given patch. Remember, "given enough eyeballs, all bugs are shallow" ceases to be true the moment individual reviewers become complacent.

Some things to check when reviewing code:


 * Patch aligns with project goals, and is ideally associated with a bp or bug
 * Commit message is formatted appropriately and contains external references as needed.
 * Coding style matches guidelines given in HACKING.rst
 * Patch is cohesive and not too big to be reviewed in a timely manner (some patches may need to be split to improve cohesion and/or reduce size)
 * Patch does what the commit message promises
 * Algorithms are implemented correctly, and chosen appropriately
 * Data schemas follow best practices
 * Unit and functional tests have been included and/or updated
 * Code contains no bugs (pay special attention to edge cases that tests may have missed)

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.