Mailing-List: contact mime4j-dev-help@james.apache.org; run by ezmlm
Precedence: bulk
Reply-To: mime4j-dev@james.apache.org
Received-SPF: pass (athena.apache.org: domain of apache@bago.org designates
 209.85.219.212 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <9426afb71001070434p476482e8hc429b90a0ad40b9f@mail.gmail.com>
References: <9426afb71001051133g3f4a065bsb7058cc6e2ce09dd@mail.gmail.com>
	 <1262866630.5718.0.camel@ubuntu>
	 <9426afb71001070434p476482e8hc429b90a0ad40b9f@mail.gmail.com>
Date: Thu, 7 Jan 2010 13:51:19 +0100
Message-ID: <9426afb71001070451u23e5d17ard790524c4a0a2a44@mail.gmail.com>
Subject: Re: [cycleclean] branch review and questions
From: Stefano Bagnara <apache@bago.org>
To: mime4j-dev <mime4j-dev@james.apache.org>
Content-Type: text/plain; charset=ISO-8859-1

To help reviewing here are the issues fixed or partially related to
what I did in the branch. If you open each one and look at the All or
"Subversion Commits" panel you can see the related code revisions.
Changes have an order, and most time this is not random, but the
necessary order (most time the only way to fix package dependency
cycles is to fix the design issues behind the cycles.) What could
appear a bunch of unrelated changes is instead a very related sequence
of changes. While trying to remove one cycle I analyzed the code,
changed reponsibilities to separate concerns, moved code around, found
bugs.

[just started] Improve organization of fields and fields parsers
http://issues.apache.org/jira/browse/MIME4J-145

[done] Fix Field, RawField and ParsedField consistency/confustion,
dependency hell.
http://issues.apache.org/jira/browse/MIME4J-149

[partial] Mime4J does not support mandatory rfc5322 "obsolete" syntax
http://issues.apache.org/jira/browse/MIME4J-150

[done] Ignore the first WSP in getBody, and add one WSP after the ":"
when writing (customary practice)
http://issues.apache.org/jira/browse/MIME4J-151

[done] Zero parts multipart messages are parsed as 1 empty part
multipart messages
http://issues.apache.org/jira/browse/MIME4J-152

[done] Headless inconsistency between MimeTokenStream and MimeStreamParser
http://issues.apache.org/jira/browse/MIME4J-153

[done] ContentHandler should rely on
MimeStreamParser.setContentDecoding instead of dealing with their own
transfer decoding code.
http://issues.apache.org/jira/browse/MIME4J-154

[done] MimeEntity should receive a new MutableBodyDescriptor for its
body, and not receive the optional parent.
http://issues.apache.org/jira/browse/MIME4J-155

[done] DOM (message) classes should not be implementation specific.
Move implementation to a different package (message.impl)
http://issues.apache.org/jira/browse/MIME4J-156

[partial] Reorganize code/packages/classes in order to better
self-document mime4j architecture and be able to split it into modules
http://issues.apache.org/jira/browse/MIME4J-157

[partial] Reduce usage of commons-logging in favor of a "Monitor" service.
http://issues.apache.org/jira/browse/MIME4J-158

[done] Lenient dealing with headless messages or malformed header/body
separation
http://issues.apache.org/jira/browse/MIME4J-58

[done] QuotedPrintableInputStream may be lenient with newlines but
should be consistent in decoded data.
http://issues.apache.org/jira/browse/MIME4J-161

Stefano

2010/1/7 Stefano Bagnara <apache@bago.org>:
> 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
>