james-server-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Bernd Fondermann <bf_...@brainlounge.de>
Subject Re: [jira] Updated: (JAMES-509) Cleanup/Refactor FetchMail code
Date Sun, 28 May 2006 12:13:27 GMT
Stefano Bagnara wrote:
> 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.

Agreed. That's basically what I tried to say in the next sentence:

>> 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.

"Risk" is a term depending on each persons subjective evaluation and 
very difficult to measure. Developers tend to underestimate risks of 
their changes.
What we can measure is quality. If we cannot be sure to have the same 
functionality before and after a larger refactoring, we have lower quality.

> 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?

Well I admit, that I wasn't sure about this, concerning this specific case.
Seems I was blatently wrong. My apologies.

> 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.

I don't mean to block your work. My primary concern is to improve James 
quality. Improvement includes changes to the codebase ;-) , so please go 
ahead.

> Bernd Fondermann wrote:
>> IMHO, if you find out that the touched code works like before or 
>> better, go ahead and apply the patch.

+1 to refactor Fetchmail.
+1 to thoroughly test the refactoring.

   Bernd


---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org


Mime
View raw message