incubator-cassandra-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jonathan Ellis <jbel...@gmail.com>
Subject Re: working together
Date Thu, 09 Apr 2009 01:38:08 GMT
On Wed, Apr 8, 2009 at 6:26 PM, Sandeep Tata <sandeep.tata@gmail.com> wrote:
> I think it is reasonable that a codebase that has evolved for over two
> years has significant opportunity for refactoring when it is opened to
> a host of new developers. That said, large scale refactoring *at this
> stage* hurts us in two ways:
>
> 1. We don't have a rich suite of unit tests.

True.  The solution is to add tests a little at a time, preferably
before the refactor so it can help catch regressions.

> Even automatic
> refactoring without unit tests makes me uncomfortable.

I don't know how else to say this: that's an unwarranted overreaction.

> 2. Big refactoring makes it difficult for the original developers
> (A&P) to review patches quickly.

That is why I break the refactoring parts into separate patches.  It
is not hard to review when changes are split up this way.

> perhaps we should hold off on accepting big refactorings

Unless we have vastly different ideas of what "big" means (which is
possible), I emphatically disagree.

The important question is not, "Is refactoring more dangerous than
doing nothing?"  Of course it is.  Writing code is always more
dangerous than not writing code.

The real question is, "Is adding new features while refactoring more
dangerous than adding new features without refactoring?"  And the
answer is no.  Refactoring improves the fit of the code to its current
purpose rather than grafting layers of new features on top of a
foundation that was not meant to support them.  Refactoring also helps
prevent bugs -- for example my remove patch introduced a bug fixed in
r761093 -- because I only modified one of three copies of a piece of
code.  It can also expose bugs in existing code.  See CASSANDRA-58 for
an example of a bug that Jun Rao noticed because of a refactoring
patch.

-Jonathan

Mime
View raw message