2010/1/8 Oleg Kalnichevski <[email protected]>: > 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
