Trevor Parscal wrote:
On 11/2/10 12:40 PM, Roan Kattouw wrote:
I agree that review and deployment are two parts of a whole and that one doesn't make sense without the other, but I don't think that, of the two, deployment is necessarily the defining part.
Do you mean that there are times we would want to review something that would not be later deployed?
- Trevor
Reviewing means 'the code has been looked [at some user dependant scrutiny level] by someone other than the author and deemed to be right/acceptable/bug-free'.
A user provided patch in bugzilla needs reviewing before adding to trunk*, but that doesn't mean it should be deployed (trivial example: it could be a patch for an extension). A revision on a non WMF-used extension or a branch can be reviewed. If it has passed the process we defined as reviewing (and perhaps had some bugs fixed in doign that), there is no sense to mark is as not reviewed. Sure, the extension/branch may later have a full review if it is going to be deployed. But they should be marked as reviewed if they are. In a perfect world, every bit of code in trunk would be reviewed. That would make for a really sane software. For now, let's struggle for reviewing truunk phase3 and the long dreamed periodical scaps.
*In fact, that would have a double review, first by the commiter and then by the revision reviewer, unless it is deferred.