james-mime4j-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stefano Bagnara <apa...@bago.org>
Subject Re: [cycleclean] branch review and questions
Date Thu, 07 Jan 2010 12:34:09 GMT
2010/1/7 Oleg Kalnichevski <olegk@apache.org>:
> I am very sorry, Stefano, but I am not in favour of a _wholesale_ merge
> of your changes. You have touched too many things in too many places.
> The extent of changes is well beyond the initially stated goal and the
> scope of the refactoring, which was meant to be elimination of cyclic
> dependencies.

I don't agree. I started the branch to refactor the whole mime4j. The
current content is only a partial result of my real goal. The name
"cycleclean" is only a label, don't be fooled by it.

> You have introduced a number of significant _functional_
> changes at the same time, which makes it difficult to understand the
> intent of changes.

IMO every change I introduced is an improvement. I translate your
"introduced a number of significant _functional_ changes" as "fixed
several bugs and design issues" ;-) . Feel free to review them one by
one. I committed them separately. Most of them are not applicable if
you don't get the whole thing, because to fix one thing I had to fix
other things previously.

> Overall, I cannot help feeling that your efforts lost
> focus. Things did not really get better, they just got different, more
> to your personal taste.

> For instance, how does impl class
> MimeTokenStream extending public class BasicMimeTokenStream make for a
> better API, I just do not understand. So, cyclic dependencies are gone
> but in return we have ended up having some really bizarre class
> extensions.

>From an API perspective you should not care what MimeTokenStream
extends ;-) From an internal design choice the fact is that
BasicMimeTokenStream does not have dependencies against the rest of
mime4j, while MimeTokenStream does the linking to the rest of the
components.

> I personally prefer to have your changes merged in several iterations,
> package by package, starting with low level stuff in 'parser' and 'io'
> packages. Feel free to ignore me, though.

It is NOT possible. Just look at the changelog. Every step is very
well explained in the commit and in the relative JIRA issue.

That's said, is up to you. I don't want to force any of my personal
preferences in mime4j. I'm really convinced that the branch is a BIG
STEP forward for mime4j, but not the complete step that is needed. And
keep in mind that removing cyclic dependencies was not a goal for the
branch, but a needed requisite to refactor and be able to implement
the stuff I documented with JIRA issues (like having a DOM with no
dependencies).

If you (all of mime4j committers) like it, then we merge it, otherwise
I don't think I can make much more than this. It cannot be any simpler
to fix design choices and the many different hands that worked on
mime4j.

Mime4j 0.3 and 0.4 was much better to my eyes, in this regard, and
some of the changes introduced in 0.5 and 0.6 had complicated the
whole design, again IMO. I was not able to introduce the features I
need in the current trunk as I can't really grok it so the branch was
the place for me to simply do the work, fix the issues (to my eyes, of
course) and speak with code instead of messages. In current trunk
there is no coherence and when using mime4j for anything but the
simpler parsing operation I had to start subclassing mime4j classes.
Cleaning up the whole thing will take much more that what I wrote in
the branch.

If you need explanations about some of that code just ask.

Stefano

Mime
View raw message