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(a)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.…
Sent from the Wikipedia Developers mailing list archive at
Nabble.com.