Steve Brewin (JIRA) schrieb:
For me, this is a prime example of why people have said we need to discuss
first, achieve consensus, then act.
There is a patch, that we can discuss. If it makes sense it should be
included.
I'm afraid I don't understand why this wasn't discussed first on the lists. Is
it something we need to do? What is the motivation for such a major
refactoring? We all know that major changes are prone to creating new issues.
What functional issues are being solved? Are they of sufficient importance to
outweigh the risk of destabilisation such a radical change engenders? What
additional features are provided?
The argument for change seems to be "Current FetchMail code is really hard to read and
manage". Others have commented how well structured it is (I don't have time to look up the
references). This is a matter of style. I'm biased as I refactored the original code into the
structure you see now and added a heap of functionally. I don't want to get into an argument about
style or how virtuous having "removed 2200 lines of code (50Kbytes of code)" is.
I think that the changes are so radical that we are in effect replacing the current implementation of fetchmail and need to ask why? Unlike when I refactored it, it isn't broken and no new features are added so we are not satisfying any user requirements.
In making these changes, without even a safety net of unit tests, we are
destabilising a component for no more than a matter of style. To me, this
doesn't seem wise. There are no obvious benefits that outweigh the risks.
I think saving time is a very serious argument. This is my own
experience: Sometimes there are components which aren't subject of big
changes but you always have to go through of them and maybe make small
changes. And when the code is hard to read and hard to understand it
costs a lot of time. At the end you could have rewrite it from scratch
and you would have saved much time and that means a lot of money in the
commercial world. Sometimes I do this step and refactor/rewrite and
sometimes I am annoyed that I haven't done it. Yes it can be reasonable
touching code without bringing features for the end user.
Of course there are risks and it has to be discussed that things don't
get worse. So demanding unit tests maybe an argument. On the other hand
keeping hard to manage code is a risk, too, because it is error-prone
when you have to do changes.
Time isn't money in the open source world although time of volunteers it
the most valuably. We should not waste it.
I haven't had my hands in the fetchmail code so I only can do a fast
review. And the patch makes sense to me. For example there are getters
an setters for variables only used locally in one method. If you don't
have them local you always have to check for references and side effects
when reading that code. IMHO it looks a bit over-designed to me.
Joachim
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
