On 11-07-26 01:27 PM, Chad wrote:
While spending the past few days/weeks in CodeReview, it has become
abundantly clear to me that we absolutely must get away from this idea
of doing huge refactorings in our working copies and landing them in trunk
without any warning. The examples I'm going to use here are the
RequestContext, Action and Blocking refactors.
We've gotten into a very bad habit recently of doing a whole lot of work in
secret in our respective working copies, and then pushing to trunk without
first talking to the community to discuss our plans. This is a bad idea for a
bunch of reasons.
Firstly, it skips the community feedback process until after your code is
already in trunk. By skipping this process--whether it's a formal RfC, or just
chatting with your peers on IRC--you miss out on the chance to get valuable
feedback on your architectural decisions before they land in trunk. Once code
has landed in trunk it is almost always easier to follow up and continue to
"fix" the code that should've been fully spec'd out before checkin.
Also, the community *must* have the chance to call you crazy and say "don't
check that in, please." Going back to my examples of Actions, had the community
been consulted first I would've raised objections about the decisions made with
Actions (I think they should be moved to special pages and the old action urls
made to redirect for back-compat...rather than solidifying the old and crappy
action interface with a new coat of paint). Looking at RequestContexts, had we
talked about this in an RfC first...we probably could've skipped the whole
member variable vs. accessor debate and the several months of minor cleanups
that's entailed (__get() magic is evil, IMHO)
Secondly, this increases the load on reviewers, myself included. When you land
a huge commit in trunk (like the Block rewrite), it takes *forever* to review
the original commit + half a dozen or more followups. This drains reviewer time
and leads to longer release cycles. I think I speak for everyone when I say this
is bad. Small incremental changes are infinitely easier to review than large
If you need to make huge changes: do them in a branch. It's what I did with the
installer and maintenance rewrites, what Roan and Trevor did with ResourceLoader
and what Brian Wolff did with his metadata improvements. Of course after landing
your branches in trunk there will inevitably be some cleanup required, but it
keeps trunk more stable until the branch merge and makes it easier to back out
if we decide to scrap the feature/rewrite.
I know SVN branches suck. But the alternative is having a constantly unstable
trunk due to alpha code that was committed haphazardly. Nobody wins in that
So please...I beg everyone. Discuss your changes first. It doesn't have to be
formal (although formal spec'ing is always useful too!), but even having a
second set of eyes to glance over your ideas before committing never hurts
RequestContext WAS an RFC.
No one commented on it...
When it was originally committed it was a small change. It just grew as
people improved it. It's not a large change like ResourceLoader and
Installer that gets everyone's attention.
Frankly if RequestContext and features of similar size had been
committed to a branch instead of core, they would have bit-rotted there
and never made it into core. Not to mention the relative size compared
to things like ResourceLoader and Installer makes the negatives of svn
branches relatively worse. Dozens of med-small changes getting shoved
into branches and atempted to be merged into core?
I'd love to do work in branches, have everyone actually try the branches
out and add their improvements there, then get them reviewed into trunk.
But that workflow... that's really git, not this svn mess we have now.
Even if we kept this in the environment of svn and tried to use RFCs...
people would have to actually discuss those RFCs... looking over the
RFCs we have it looks like only 10% of them even get anyone discussing
them. The rest just bit rot... So we're supposed to move from having a
volunteer willing to code a feature and putting it into trunk, to having
them putting up an RFC or putting it into a branch, and letting the code
bit rot or RFC just sit there forever, and neither ever get into core?
~Daniel Friesen (Dantman, Nadir-Seen-Fire) [http://daniel.friesen.name