[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kmail-devel
Subject:    Re: [PATCH] ClientInterface (next try)
From:       Don Sanders <sanders () kde ! org>
Date:       2003-08-08 12:52:21
[Download RAW message or body]

Hi Andreas,

Thanks for your work. I've been running with this patch applied for a 
couple of days without any problems.

Basically the patches look ok to me, and the notifier/client split 
makes sense. I notice a few small things.

In client.cpp I guess the commented out:
KMail::Client::slotSetStatusMsg should be removed, and the same for 
the declaration in client.h right? Or is that not finished yet?

What needs to be done before +#include "client.h" can be removed from 
kmsender.cpp?

Not sure what the aMsg->setComplete( true ); is for in 
KMSender::send, maybe you can clarify that.

Are the places in kmsender.cpp that are marked:
// TODO  - this needs some more effort to separate kernel and GUI
& // TODO - this needs effort to separate kernel and GUI
the only place where kmsender uses GUI code directly now?

Don.

On Thursday 07 August 2003 07:06, Andreas Gungl wrote:
> Okay, some improvements (although I didn't solve the two big issues
> with the dialogs yet):
>
> - Namespace KMail is used for the new classes.
>
> - I cleaned up the diverse statusMessage() variants in kmsender. I
> inspected the code carefully but didn't test all variants. At least
> I'm convinced the methods in kmsender are no longer needed.
>
> - I introduced a new class Notifier which now holds some
> declarative elements needed by the Client class. This switches the
> dependencies so that client classes are dependent on the kernel
> classes but NOT the other way round as in my first patch. IMO
> Notifier is a kernel (helper) class.
>
> - Coupling between Notifier and Client is done via signals/slots.
> Further extensions can be made easily (even the DCOP way if ever
> decided to go for that).
>
> I'm not sure who is interested in the details of this change.
> Nevertheless I want to give a chance to object before I commit my
> work to CVS (when I have finished kmsender - but feedback is
> welcome already now).
>
> Andreas
>
> On Sunday 27 July 2003 00:19, Cornelius Schumacher wrote:
> > On Sunday 27 July 2003 00:16, Andreas Gungl wrote:
> > > Any comments, hints and whatever are welcome as everytime.
> >
> > Just a little nitpicking, I can't comment on the content of the
> > patch, because I didn't really look at it:
> >
> > - Instead of naming the class "KMailClient" it might be better to
> > name it "Client" and put it into the "KMail" namespace. This has
> > the same effect, but makes the code a little bit clearer inside
> > of KMail.
> >
> > - The include guard should better be "#define KMAIL_CLIENT_H"
> > instead of "#define CLIENT_H". Chances that KMail sometimes
> > includes another CLIENT_H somewhere aren't high but probably
> > above zero.
_______________________________________________
KMail Developers mailing list
kmail@mail.kde.org
http://mail.kde.org/mailman/listinfo/kmail
[prev in list] [next in list] [prev in thread] [next in thread]