[Kde-pim] Moving MessageStatus to kdepimlibs

[Sebastian Kügler]

Hi Leo,

Thanks for the merge-into-libs effort of this class. :)

On Thursday
29 July 2010 14:45:22 Leo Franchi wrote:
> Reviewboard won't accept my patch
(apparently it doesn't like changes
> to files that are moved), so i'm
posting it here as I'd like some
> review.
> 
> MessageStatus is in
kdepim/messagecore/, which (a) makes it hard to
> use elsewhere and (b)
results in it being copied all over. I know
> there are a few copies
floating around and I just removed an
> out-of-sync copy from the
kmindexreader library. There is also one in
> the mixedmaildir, and also in
lionmail, for example. It's useful as
> any app that deals with
kmime:messages and wants to interact with
> flags will want to use it.
> 
>
Also, as i'm planning to add attachment status support for messages, i
>
would like to use the attachment-related flags from MessageStatus in
> the
resources, which also requires MessageStatus to be in pimlibs or
> copied.
>

> In the attached patch, I moved MessageStatus to
>
kdepimlibs/akonadi/kmime/. The new namespace is
> Akonadi::MessageStatus,
because I couldn't think of a better one. I
> know pimlibs has stricter API
requirements, hence the request for a
> review. Patch is
attached.

Generally, yes, please make this sharable, it's needed to keep
things sane in Lion Mail, and the little bit of logic that's in there is not
fun enough to write so that I'd do it again. Some remarks while using this
class (all no biggies, I think it's quite fine as it is, but before becoming
public API, let's at least consider):

* While using it, I noticed some
warts in the API though, there are methods  
  for isNew(), isUnread(),
isRead() and then setOld(), setRead(), setUnread()

* setToAct() also sounds
weird, I'd use setTodo(), setTask() or 
  setActionItem().

* setHam() and
setSpam() might also be compressed into one method (it takes a 
  bool
already anyway, and I don't see semantic differences between 
 
setSpam(false) and setHam(true). Hm, maybe the "it's neither" case?

* The
method setHasAttachment(bool withAttachment) sounds weird, maybe 
 
withAttachment -> hasAttachment.

* The APIDOX should probably mention that
the new status is not synched to 
  Akonadi by this class, it's just for
internal bookkeeping.
-- 
sebas

http://www.kde.org | http://vizZzion.org |
GPG Key ID: 9119 0EF9
_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/