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] minor naming tweaks; was Re: [cycleclean] branch review and questions
Date Fri, 08 Jan 2010 10:19:51 GMT
2010/1/8 Oleg Kalnichevski <olegk@apache.org>:
> I spent more time working with your code. The refactoring of
> MimeTokenStream makes good sense. However, the unfortunate choice of
> package and class names sent a completely wrong message.

Happy to hear this! So, maybe I'll take the time to apply some package
name changes to let the correct message pass throught.

> It led me to
> believe one package was meant to represent public API with impl package
> being implementation of that API. If the following changes can be made
> to package / class naming I can happily vote +1 to merging down the
> entire branch. Smaller issues such as #unread() method modality can be
> dealt with on the trunk.
>
> option 1)
> o.a.j.mime4j.parser -> o.a.j.mime4j.stream
> o.a.j.mime4j.parser.impl -> o.a.j.mime4j.parser
> impl.MimeTokenStream -> MaximalMimeTokenStream

As I wrote previously the "package names" I chose and "their nesting"
is not something I took into consideration in the branch. Also i
didn't care of "class names". I only worked on "packages content",
"packages dependencies" and some fix/feature.

I'm fine with the package rename you propose here, I'm not sure why
you propose to rename MimeTokenStream to MaximalMimeTokenStream as we
could leave the old name and the old package to let a smoother upgrade
for users. MimeTokenStream in the branch did not remove old
entrypoints (I simply added some constructor, IIRC). BTW I'm not
against renaming also MimeTokenStream.

> option 2)
> o.a.j.mime4j.parser.impl -> o.a.j.mime4j.parser.max
> impl.MimeTokenStream -> max.MaximalMimeTokenStream

I prefer option 1 very much.

> I also played around with idea of the iterative merge and found it
> pretty doable. I merged down low level packages util, io, parser, codec
> and managed to tweak higher level packages to compile against the new
> parser API. Merging your code in stages would make it possible for
> people with better knowledge of higher level API review the changes
> without being distracted by changes in low level stuff. I am perfectly
> aware of downsides of this approach, such conflict resolution pains, but
> still feel it could help build a consensus among all committers.

We are not talking of diverging branches to be merged. We are talking
of a sequences of revisions applied in a branch while trunk was
stopped, so I really don't understand why we should rewind and play
back the whole thing altering the order of changes. It's just a matter
of really looking at revisions, as if they was happening in trunk.

I've been very careful to make clean changes trying to keep diffs as
small as possible because I really know that reviewing is a difficult
task (I spend more time reviewing than coding :-( ).

Also, I don't know what you put in "low level" and in "higher level",
so it would be very hard for me to do that.

If you think you know how to do it and you think it useful then simply
go ahead. Anything that can bring some of my code to trunk is a +1
from me :-)

Stefano

Mime
View raw message