Also, with some coordination, branches could be merged at a time when: * Reviewers will looking at it and testing it *right* after the merge (in addition to any branch review) * The author is around to make fixes as it gets final review
People should try to be available after any large changes (from a branch or from their local code). However, it may be the case that no one has the time to specifically focus on reviewing a certain change set right after commit. Maybe, for example, changes will only get seriously looked at a month after the commit. If the author was available in the weeks after the commit, but not when it gets serious review, then we still have a problem. This is also were some advance notification and coordination could help (especially looking at the examples Chad gave). ^demon 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 > > _______________________________________________ > Wikitech-l mailing list > Wikitech-l@lists.wikimedia.org > https://lists.wikimedia.org/mailman/listinfo/wikitech-l > > -- View this message in context: http://old.nabble.com/On-refactorings-and-huge-changes-tp32143260p32143408.html Sent from the Wikipedia Developers mailing list archive at Nabble.com. _______________________________________________ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
