incubator-cassandra-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Zhu Han <schumi....@gmail.com>
Subject Re: working together
Date Thu, 09 Apr 2009 06:29:14 GMT
[so many mistakes without spelling check, sorry for it]

Jonathan,

I can understand why you refactored the code a lot in the last few month.
And I saw you were working hard to improve it  in the last few months.

However, the talents from Facebook have done a lot of work to bring
Cassandra to the world. And they have deployed it to the production system
for some time. Any type of patch could bring instability to the system,
which is very critical for the production environment.  Why not freeze
refactoring the code before the community can bring a whole bunch of unit
test case of integrate test case and committed it into the code base?  The
Facebook team doesn't have enough time to do it. That's what the community
can contribute to the project and it should be the top item on the TODO
list.

If all the test cases are ready, the regression bug could be detected
easily. Whatever, even the most trivial patch could bring an unexpected bug
to the system.  Most reviewers would not review the patch seriously, just
because it's so trivial, or the patch cannot show the context clearly, and
finally, we may not understand the code thoroughly. Let's depend on the test
case to fight against this type of bug.

Avinash,

The overall architecture and implementation of Cassandra is very clear and
impressive. But the refactoring is still necessary because it would bring
the code quality to a higher layer. But we should take it more seriously and
more cautious, should we?

best regards,
hanzhu


---------- Forwarded message ----------
From: Zhu Han <schumi.han@gmail.com>
Date: Thu, Apr 9, 2009 at 2:24 PM
Subject: Re: working together
To: cassandra-dev@incubator.apache.org


Jonathan,

I can understand why you have refactored the code a lot in the last few
month.  And I saw you were working hard to improve it  in the last few
months.

However, the talents from Facebook have done a lot of work to bring
Cassandra to the world. And they have deployed it to the production system
for some time. Any type of patch could bring unstability to the system,
which is very critical for the production environment.  Why not freeze
refactoring the code before the community can bring a whole bunch of unit
test case of integrate test case and commited it into the code base?  The
facebook team doesn't have enough time to do it. That's what the community
can contribute to the project and it should be the top item on the TODO
list.

If all the test cases are done, the regression bug could be detected easily.
Whatever, even the most trivial patch could bring an unexpected bug to the
system.  Most reviewers would not review the patch seriously, just because
it's so trivial, or the patch cannot show the context clearly, and finally,
we may not understand the code thoroughly. Let's depend on the test case to
fight against this type of bug.

Avinash,

The overall architecture and implementation of Cassandra is very clear and
impressive. But the refactoring is still necessary because it would bring
the code quality to a higher layer. But we should take it more seriously and
more cautious, should we?



best regards,
hanzhu



On Thu, Apr 9, 2009 at 9:38 AM, Jonathan Ellis <jbellis@gmail.com> wrote:

> 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
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message