[GSOC] Hbase Mailbox code review

[Norman Maurer <no...@apache.org>]

Hi Ioan,

I did take the time to review your code and It seems to be in a very 
good shape :)

I just found some minor things that should porlly get addressed:

* Merge AbstractHBaseMessage with HBaseMessage ( I see no point to have 
them seperated)
* Move the merged version to the package 
org.apache.james.mailbox.hbase.mail
* Remove the overriden method createMessage(...) from 
HbaseMessageManager as it only calls super.crwateMessage(..)
* HBaseUtils.messageMetaFromResult(..) needs to set the modseq and the 
textualLineCount on the returned HBaseMessage
* HBaseSubscription could get removed and just use SimpleSubscription. 
This change will allow to also remove HBaseSubscriptionManager as you 
can just use StoreSubscriptionManager, so one class less to worry about..
* HBaseMailboxMapper.save(..) and HBaseMailboxMapper.delete(..) should 
use the paramized version of Mailbox as paramater (Mailbox<UUID>)
* HBaseMessageMapper has unused imports..


More comments will follow later ;)

Thanks,
Norman





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

Re: [GSOC] Hbase Mailbox code review

[Eric Charles <er...@apache.org>]

Hi,
Yes, very good work Ioan :)
+1 on Norman comments (sounds like 'Less is More').

I guess you are focusing on integration tests for the final rush 
(deadline is approaching).
We could review the tablepool, the indexes on datamodel... later after gsoc.

Thx.

On 16/08/11 09:26, Norman Maurer wrote:
> Hi Ioan,
>
> I did take the time to review your code and It seems to be in a very
> good shape :)
>
> I just found some minor things that should porlly get addressed:
>
> * Merge AbstractHBaseMessage with HBaseMessage ( I see no point to have
> them seperated)
> * Move the merged version to the package
> org.apache.james.mailbox.hbase.mail
> * Remove the overriden method createMessage(...) from
> HbaseMessageManager as it only calls super.crwateMessage(..)
> * HBaseUtils.messageMetaFromResult(..) needs to set the modseq and the
> textualLineCount on the returned HBaseMessage
> * HBaseSubscription could get removed and just use SimpleSubscription.
> This change will allow to also remove HBaseSubscriptionManager as you
> can just use StoreSubscriptionManager, so one class less to worry about..
> * HBaseMailboxMapper.save(..) and HBaseMailboxMapper.delete(..) should
> use the paramized version of Mailbox as paramater (Mailbox<UUID>)
> * HBaseMessageMapper has unused imports..
>
>
> More comments will follow later ;)
>
> Thanks,
> Norman
>
>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
> For additional commands, e-mail: server-dev-help@james.apache.org
>

-- 
Eric

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