james-mime4j-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Oleg Kalnichevski <ol...@apache.org>
Subject Re: [cycleclean] branch review and questions
Date Thu, 07 Jan 2010 13:32:12 GMT
On Thu, 2010-01-07 at 13:34 +0100, Stefano Bagnara wrote:
> 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.

Of course, why should I care? MimeTokenStream extending
BasicMimeTokenStream makes perfect sense, doesn't it?

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

Isn't it great that now we have 'all or nothing' decision to make? 

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

Every time I raise a concern you basically say you know better, like
when I complained about really bizarre contract of
LineReaderInputStream#unread() method.

I stated my opinion. Feel free to ignore it. If committing all these
changes is the price to be paid for your participation in further
development of mime4j, I guess it is the price worth paying. I just do
not think it is okay to have made lots of API changes all over the
project and leave us with a decision to choose all or nothing.

Oleg 



Mime
View raw message