Only some people (project owners, gerrit admins, some WMF staff, etc.) can give CodeReview+2 (approved), whereas everyone can give CodeReview+1. Only people able to approve can mess with Verified I think...

IMO leave it to Jenkins, if Jenkins is set up for the project. But if you want to use it to indicate that you ran the unit tests yourself and/or extensively tested it manually, feel free I guess as long as it doesn't confuse Jenkins.

My understanding is you *should* use it. Jenkins only does V+1, which is "Checked". V+2 is required for some/all repos (e.g. extensions). Normally, the original developer at least should provide Verified (since they should have tested their own code).

I think just how certain you are.

+2 means it's ready to merge. In core, this will cause unit tests to run, and if they pass, it will automatically merge. I don't know of any reason (in any code) to vote CR +2 if you don't think it's ready to merge.

On 12/27/2012 10:31 AM, Matthew Flaschen wrote: I still don't get it. I either think the code is good and should be merged or it's not good enough and shouldn't be merged. I don't see any situations in between, but maybe it's just me ;)

This still seems redundant. If Jenkins runs tests only when I give +2, but I am supposed to give +2 only if I already run the tests myself manually, then what's the point?

On Dec 27, 2012, at 7:18 PM, Juliusz Gonera <jgonera(a)wikimedia.org> wrote: == 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. _______________________________________________ Wikitech-l mailing list Wikitech-l(a)lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l