On Dec 27, 2012, at 7:18 PM, Juliusz Gonera jgonera@wikimedia.org wrote:
Hi,
I'm a bit confused when it comes to various options I have in gerrit and it seems the docs are not up to date on that.
- What is the difference between Verified and Code Review? When would I put +1 in one of them but -1 in the other?
- What is the difference between +1 and +2, especially in Verified?
- Why do we even have +2? +1 means that someone else must approve. What does +2 mean? No one else has to approve but I'm not merging anyway, why?
It seems the docs (http://www.mediawiki.org/wiki/Code_review_guide) do not explain it.
Juliusz
== Verified ==
Verified is for linting and executing unit tests. * Verified +1 means "Checked" for lint issues * Verified +2 means "Tested" by executing the Jenkins tests
If you are a human, do not use Verified (most people don't have user permissions to set this, anyway).
If you "tested" the change (either by checking it out locally and using the wiki and/or by running phpunit locally) and you have the ability to set Verified, still do not set it.
It does not mean the same thing. Because the Jenkins tests are much more elaborate than the testing you may do locally (e.g. different database backends, and soon also different browsers and test suites: jshint, phplint, phpunit, qunit, mysql, sqlite etc.).
We might rename this field to "Automated Testing" for clarification ("Verified" is a generic name that is the default in Gerrit) and (where not already) it will eventually be restricted to bots only[2].
== Code Review ==
Code Review is for human review of the code.
A positive value means you believe it is perfect and may be merged as-is (under the condition that the Jenkins test will pass[1]). If you have merge rights then you'd give +2. Others would give +1.
A negative value means there are issues.
[1] So if you set CR+2 you're saying "The code looks great, let Jenkins run it, and if it passes, merge it". [2] Except for wmf-deployment probably, to be able to override it if Jenkins is having issues and certain commits need emergency deployment.