On Tue, Nov 2, 2010 at 7:39 PM, MZMcBride <z(a)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