james-server-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stefano Bagnara <apa...@bago.org>
Subject [mime4j] packages / MIME4J-51 (Was: what else need to be done)
Date Thu, 07 Aug 2008 09:52:28 GMT
Niklas Therning ha scritto:
> Stefano Bagnara wrote:
>> Niklas Therning ha scritto:
>>> Stefano Bagnara wrote:
>> [...]
>> Here is an updated summary of what are our current proposed solutions:
>>
>> A) vote down MIME4J-51 and revert the code change.
>>    + least surprise
>>    + no upgrading effort required
>>    * parser is in main package
>>    - not good package classification
>>    - cyclic dependencies
>>
>> B) move mime4j.* to mime4j.core.* and mime4j.parser.* to mime4j.* (so 
>> to bring parser classes in the main package).
>>    + remove cyclic dependencies
>>    + better packaging
>>    * parser is in main package
>>    - BodyDescriptor and MimeException are in a different package than 
>> before (upgrading issue)
>>    - MaximalBodyDescriptor is a different package than before 
>> (upgrading issue, at least for james-server)
>>
>> C) leave everything as is in trunk (mime4j-51 applied!).
>>    + remove cyclic dependencies
>>    + better packaging
>>    + better organization to host non parsing related code, too.
>>    * parser is in the parser package
>>    - most "public" classes are moved around (upgrading issue)
>>    - main parser classes are in mime4j.parser instead of main.
>>
>> D) move ContentHandler, AbstractContentHandler, MimeStreamParser, 
>> MimeTokenStream from parser to main:
>>    + better packaging
>>    * parser is in main package
>>    - cyclic dependencies: introduce 6 new package cycles:
>>      main => parser => descriptor => field.datetime.parser => main
>>      main => parser => descriptor => field.mimeversion => main
>>      main => parser => descriptor => field.structured => main
>>      main => parser => descriptor => field.language => main
>>      main => parser => descriptor => main
>>      main => parser => mime4j
>>    - MaximalBodyDescriptor is in a differet package (upgrading issue)
>> [...]
>> This is what I think should be used as starting point for a POLL 
>> unless we find a better agreement, so, before going to a POLL (given 
>> that Bernd say we should not use polls), can you tell if #3 would work 
>> for you by fixing the "parser centrality" issue but not fixing the 
>> "upgrading issue" ?
> 
> I'm not following you here. Do you mean "C" when you say #3?

My bad!! The mail refactoring before posting gone wrong! I need explicit 
type casting so refactoring can take care of updating my mail 
automatically :-P

#3 should have been "B"...

>> The "D" option (your proposal) was not on discussion previously so I 
>> can only tell what is my current understanding of what people would 
>> support. Where I don't put more names is because I don't yet 
>> know/understand their position wrt the given option. Of course this is 
>> my personal feeling and while I only do my best to understand people I 
>> may be wrong in interpreting ideas, so everyone is welcome to fix my 
>> understanding.
>>
>> A. -0 bago
>> B. +0/+1 robert/oleg/bago/bernd.
>> C. +1 bago/robert -0.5 bernd/niklas
>> D. -0.5 bago, +1 niklas
>>
>> If you like "B" I think (based on the above "table" and 
>> considerations) we could find consensus there, otherwise I would 
>> prefer to go the poll way so we test for real what people think and we 
>> don't have to go through my understanding.
> 
> I've thought about these options and I think I can actually live 
> happily! :) with "C". I understand why you want the parser package and 
> after giving this a bit more thought I think it would be a good way of 
> organizing the code. We will have to add the package documentation for 
> the main package as Bernd suggested.

I added an overview.html file. I thought this is more appropriate than a 
package documentation, ATM. It simply have some paragraph from our 
website home including a link to the parser/MimeStreamParser and the 
message/Message classes.
I'm not so good in english and I don't have the same mime4j knowledge 
you may have, so please look at it and make any change you think is 
needed :-)  (or write them here and I'll take care of the commit)

Here (if last build on hudson is working) you see how the generated 
javadocs looks like:
http://hudson.zones.apache.org/hudson/job/mime4j-trunk/ws/trunk/target/site/apidocs/index.html

> I have two requests when it comes to "C":
> 
> * move the *Descriptor classes from the main package to the descriptor 
> package. It doesn't make sense to have a package named descriptor and 
> then having some classes, so closely related to what's inside this 
> package, outside of it. AFAICS this doesn't introduce any cycles. Please 
> verify this because my JDepend skills are terrible. :)

You mean moving "BodyDescriptor", "ContentDescriptor" and 
"MutableBodyDescritor" from main to descriptor, right? This sounds 
great, no new cycles and indeed a better "class-tree" to "package" 
classification!

I already committed it :-)

Here is the current package tree:
http://people.apache.org/~bago/mime4j/mime4j-51/graph-mime4j-package.gif

> * Rename the stream package io. MimeTokenStream is a stream too. It's a 
> bit confusing to me that it isn't in the stream package.

I agree.

I just committed a refactoring to make most of them extensions of 
FilterInputStream as they all of them take an inputstream in the 
constructor, so maybe streamfilters is the best one... I didn't convert 
EOLConvertingInputStream because it introduce one more filter in between 
and it's less easy to update, but it is a filter in facts.

I'm fine with "io" but maybe a better option would be 
"streamfilters"/"inputstreams"/"inputfilters"/"filterinputstreams"/"filteris" 
so to make it more descriptive. Opinions?


In the mean time I also created a message.storage package we discussed 
in past so to remove some more utility classes from the util package.
ATM I have no solution (to the concern raised by Bernd and Robert) for 
the remaining utils because each of them is used by almost every other 
package we have:
http://people.apache.org/~bago/mime4j/mime4j-51/mime4j-utils.gif

Stefano

---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org


Mime
View raw message