On Tue, Nov 2, 2010 at 7:39 PM, MZMcBride z@mzmcbride.com wrote:
Rob Lanphier wrote:
After review, some (but not all) of the features in the review queue then need to be reviewed for checking into the deployment branch. Our short term answer to that was the deployment queue: http://www.mediawiki.org/wiki/Deployment_queue
I have a few comments.
You're chomping at the edges again, but not focusing on the larger issue. It's still about _WHO_ is going to be deploying these extensions (or doing general code updates). That question is still unresolved and it's at the heart of the problem. By focusing on the periphery, you're simply needlessly adding paperwork (like the new "Deployment queue") without actually accomplishing anything. Read on for a specific example of why deployment is sometimes the sole issue, not review.
+1. I feel like we're trying to change a workflow when we should be trying to get rid of a backlog. Instead of trying to discuss ways to improve the workflow, we should Just Do It.
For example, I don't agree that the code review workflow is a bit awkward. However, when Tim had to leave and we needed people to step up reviewing--we just stepped up; we didn't discuss how to first improve the process.
Likewise, I think we should first focus on enabling more people to do merge+scap (maybe the same group who review code?) and get it going on a more often basis.
Having a good workflow is great, but I think we should look at the bigger issue first. We can design the best workflow ever, but like code review if only one person does it it all falls apart.
The issue with focusing on a "Review queue" is that it puts the focus and emphasis in the wrong place. Nobody cares about getting their code reviewed (per se). They want their code live on the site. Most developers don't care if their code is efficient or not (Domas can speak more to this), they just want to see their hard work in production. That's why they contribute code (or contributed, as the case seems to be nowadays). So the emphasis should be on deployment, a key step of which is code review, to be sure. But putting the focus on review (as though once review is finished, something will magically change) is silly and a poor idea.
IMHO: If minor** code has been reviewed and deemed fit for use, it should be merged almost immediately. Full stop.
** by minor, I mean as long as it doesn't create dependency issues, require schema changes, or are a huge refactor.
-Chad