Bernd Fondermann wrote:
Stefano,
One question: The refactoring you are providing, does it work? (Did you
run it? Did you test all the features and functionality beforehand and
afterwards?)
I already wrote I don't have run james with the refactored version: it
takes time to do this things. I'm willing to do that if I don't have vetoes.
I agree with Steve that a refactoring is not worth applying if we don't
know if it doesn't breaks something, regardeless how simple the
refactoring is, how ugly the code was and how shining it is afterwards.
I made this mistake too many times before myself.
I big -1 to this: *every* refactoring could potentially introduce a new
bug. *Every* code change could potentially introduce bugs. The meter
should not be that: the question should be does it worth the risk or not.
We may discuss wether this worth the risk or not, but I don't think that
we can say "we don't know if this refactoring break something so we
don't include it". This would be the same as "we don't accept
refactorings in our process". Even if you have 100% coverage tests when
you change a single char in your source code you can't be 100% *sure*
you are not introducing new bugs.
Of course, we cannot catch every newly introduced bug at first commit,
but we should try. It's much less effort than hunt it anytime later.
Please note that the attachment was only a proposal, work in progress,
untested, only because I knew it could possibly receive vetoes. It is
not complete work and I never tested it.
I simply said that the proposal took 2 hours: much less this discussion
is taking, and a real concrete code to talk about, so I think it has
been time well spent. If there are no vetoes on the approach, on the
kind of change I will do some tests. I don't think we have to discuss
this things. Do you really think I want to commit code randomly on the
James source tree?
In past it happened I committed non-running code because I think that
sometimes it is better to see the sequence of commit about a given
refactoring than apply the final, tested result. When I'm 100% sure I
can achieve a better result and I'm confident I will not receive vetoes
I prefer this approach (even if this keep the trunk non-working for 1-2
days), instead this time I preferred to use the RTC approach, so
unfortunately there is no track of the small changes, but I thought that
someone could not have liked if I committed the change and asked for review.
These behaviours are related to who works on the codebase: I almost
worked an year "alone" on the James codebase, so it was not so important
for me to not block other people work and so on.. I work also on
projects with dozen of active committers and I use totally different
practices.
Even if there _are_ unit tests, I see it as my duty as a committer do
run additional functional test by hand. Some things simply can't be done
with unit tests.
I agree.
IMHO, if you find out that the touched code works like before or better,
go ahead and apply the patch.
It should work like or better than before. I cannot and never be 100%
sure of this, but I think the risk worth the fact that I'll be able to
manage that code later.
If I am the only one that has problem working on the fetchmail code I
agree that we should not do the step, but I would also see someone else
working on that code.
Stefano
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
