On 11-07-26 01:27 PM, Chad wrote:
All,
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 batch changes.
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 scenario.
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 anyone.
-Chad
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]