james-server-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Niklas Therning <nik...@trillian.se>
Subject Re: [mime4j] what else need to be done (Was: Benchmark of 0.3 vs 0.4)
Date Thu, 07 Aug 2008 07:03:33 GMT
Stefano Bagnara wrote:
> Niklas Therning ha scritto:
>> Stefano Bagnara wrote:
>>> Robert Burrell Donkin ha scritto:
>>>> On Tue, Aug 5, 2008 at 6:16 PM, Bernd Fondermann
>>>> <bernd.fondermann@googlemail.com> wrote:
>>>>> On Tue, Aug 5, 2008 at 18:52, Stefano Bagnara <apache@bago.org>

>>>>> wrote:
>>>>
>>>> <snip>
>>>>
>>>>>> IMHO this one (use the new structure but move some class from 
>>>>>> parser to
>>>>>> main) is the worst of the 5 analyzed, but I won't veto it, so if

>>>>>> this is
>>>>>> what the majority wants, I'm fine with it.
>>>>> I don't think that Apache is about majority decisions in the first
>>>>> place. Majority decisions (votes) are often not including minority
>>>>> opinions, while finding consensus is about taking every opinion into
>>>>> acount (known as 'compromising'). If that doesn't work, votes come
>>>>> into play. (Disclaimer: The readers notion about 'The Apache Way' may
>>>>> vary.)
>>>>
>>>> i think it's important to let people know about changes like this and
>>>> assemble some kind of rough consensus before embarking. i think
>>>> there's now a consensus that these changes are broadly an improvement
>>>> but that more refinements are possible. let's just apply the proposal
>>>> and then start working on the improvements.
>>>
>>> MIME4J-51 proposal has already been applied, unfortunately what I 
>>> thought was consensus a week ago now seems something invented by me ;-)
>>>
>>> My questions to Niklas and Bernd are really to understand if 
>>> "refinements" is something that can be done on the refactored tree 
>>> or if the consensus is now too far from that and we should better 
>>> revert it and start from that.
>>>
>>> I'm scared that people think I'm pushing my proposal or pushing my 
>>> cycle dependencies issues when I just want to see something done.
>>>
>>> I hope Niklas will update us soon, without fears or bad feelings wrt 
>>> this thread, and I also hope that someone else with better 
>>> communications skills can show what's wrong in the way I try to 
>>> build consensus.
>>
>> Hi,
>>
>> I certainly understand the merits of trying to reduce cyclic package 
>> dependencies, especially for the developers developing the software. 
>> My only concern is that zero cyclic dependencies in a library such as 
>> Mime4j doesn't necessarily mean that it will be easier to use for the 
>> end user.
>
> I think mathematical methods are the only pragmatic method to classify 
> classes in packages. Otherwise we speak about personal preferences, 
> that's why I'm so used to dependency analysis.
> IMHO a dependency graph and a correct package hierarchy tells much 
> more than any documentation.
> I understand this is a matter of personal feeling/style and I don't 
> want to convince anyone, nor to be convinced by anyone.
>
>> I definitely think that most of the refactorings you've made are 
>> great. My suggestion was only to move the four classes/interfaces I 
>> mentioned to the root package. That's all. I've played around with 
>> JDepend and if I understand correctly this change results in a single 
>> cycle (mime4j <-> parser).
>
> I tried the change and I find 6 cycles, more about this at the end of 
> this message.
>
> In my experience either you care of cycles and remove all of them or 
> you don't care at all of them. There is no such thing as "reduce the 
> cycles". Either the package dependency graph is a tree or it is not a 
> tree, there is nothing in the middle.
>
> before-MIME4J-51 the main package contained core classes like 
> BodyDescriptor and MimeException used by almost all the code and every 
> package and at the same time it contain parser classes that use code 
> from every package. If you put them in the same package you introduce 
> a lot of cycles, there is no way around or compromise for this.
>
> I understand that some practice is frustrating for people not 
> embracing it and that's why I took the proposal way and that's why I 
> would happily accept to revert the change (if this will be the 
> consensus) and I won't try to convince anyone of my opinions.
>
>> My gut feeling is that this would make it easier for the end user. 
>> However, as an active committer I'm not the typical end user myself 
>> so my gut feeling could be wrong. I can definitely live with what is 
>> in trunk right now. To me it is more important that we release a new 
>> version soon. Also, at this stage of the project I think we can 
>> afford to (and probably we will) change our minds about the package 
>> hierarchy a couple of times more as the code evolves.
>
> I agree on the release: let's not link this issue to the release or 
> will make a fast/bad decision for an unrelated goal.
>
> As I'll summarize later we can easily ship a release even today 
> including pre refactoring code, we just miss the release manager and 
> this is not a matter of solving this issue. We all agree on this, so 
> no need to mix the release issue with the repackaging+cycles issues.
>
> No one vetoed any of the proposed solution, so, in fact we are 
> discussing to find the best solution, not to find an acceptable 
> solution, and this is really good. Everyone can live whatever we 
> decide, so let's collect preferences and go with what makes more 
> people not only living, but living more happy! :-)
>
>> I don't think there is anything wrong with your communication skills. 
>> I'm impressed that you are investing so much of your personal time in 
>> trying to make Mime4j a better piece of software. Don't stop doing 
>> that! :) The reason why I didn't raise my concerns earlier is that I 
>> haven't had the time to catch up until now (I just come back from 
>> vacation).
>
> Thank you!
>
> Now I better understand your point and I see we have different 
> (somehow diverging) opinions about this.
> Unfortunately for me (and trying to find a consensus) this is 
> different from Bernd interpretation, so I'll ask you some more 
> questions if you don't mind.
>
> 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)
>
> There is another option I removed:
> X) keep that in trunk for a future release and try to release r680989 
> (and reapply r682373 and r682374)
>    * this is almost the same of "A" in the short, but already define
>      a roadmap for the future (could be "B"/"C"/"D").
> I think that everyone agree that the release has the priority on 
> everything, so this is a non-option. Regardless from what we or we 
> will agree we can release r680989 and reapply r682373 and r682374 at 
> any time.
>
> 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?

>
> 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 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. :)
* 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.

/Niklas


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