On Mon, Sep 15, 2008 at 2:04 PM, Stefano Bagnara <[EMAIL PROTECTED]> wrote: > Hi Robert, > > I just did some review of the resulting imap product and I'm impressed.
it's still a little loose: the commons-lang dependency isn't absolutely required by anything other than torque > While I reviewed I created a graph for the current modules dependencies: > http://people.apache.org/~bago/imap/james-imap-module-dependencies.gif > > Maybe this graph could be included in the website once we'll have one. +1 they are really cool :-) > I have some question/comment: > > 1) Why do we have STATUSResponse and StatusResponse ? IMAP specification i've attempt to name objects from the specification but the specification re-uses the term 'status'. there's quite a lot of complexity which could usefully be pruned so hopefully one of these can be eliminated. > 2) There are 2 package cycles: > imap.message.request.base <-> imap.message.request.imap4rev1 > mailboxmanager.impl <-> mailboxmanager.util as few as that? this is an issue that i'd intended to come back and address post-M1 > I'd like them to be removed: > 2a) The imap.message.request could be mergerd into a single package (in base > there is an imap4rev1 class!). > 2b) The mailboxmanage cycle is because of UidChangeTracker. Moving to to > "impl" should fix this too. +1 dive right in > 3) I see some module is really small (seda) while other modules are really > big (codec). I graphed the package dependency tree (it is a tree only after > grouping the 2 cycles above) and it resulted that the codec module have 2 > separated hierarchies: > http://people.apache.org/~bago/imap/graph-imap-full-package-check-no-torque.gif > Does it worth splitting? possibly again, this is something i'd planned to address post-M1. M1 was intended just to be a working IMAP for james, not anything more. the code structure could do with revision. > (note that imap.message.request.base+imap4rev1 and > imap.command.imap4rev1 can be moved in both "groups"). > 3b) maybe we could even shorten package names: codec.encode => encode and > codec.decode => decode +1 probably want to shorten imapserver -> imap as well i'm not sure whether it's worthwhile including 'imap4rev1' in the package since it's the only revision in common use > 3c) another option would be to extract the imap.message package to its own > module (message). codec (or both decode/encode if it is splitted) would then > depend on this message module). If I'm not wrong processor would so depend > only on "message" and not on codec(decode/encode). a separate message module makes sense this area is a little messy and hope it can be resolved more elegantly once some of the deprecated structure's removed. > 4) What about removing the "experimental" string from seda packages? I think > the "0.1" release version makes it already "experimental". +1 feel free to dive in and patch your proposed changes in - robert --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
