-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Thank you for reviewing. Please, see my comments on the details below. On Friday 11 July 2003 09:22, Don Sanders wrote: > I've reviewed this patch. I like it, it's a big improvement. I > have some comments: > > Could you please add a note to clientIface.h stating: > > "KMailClientInterface is subject to change without notice and > not yet part of the standard KDE API." > > This is to avoid the need to keep backwards compatibility with third > parties, and make sure that we are free to change this API at will. > Initially I would like to restrict the use of this interface to > kde-pim. > > I would prefer to keep such a notice until some time in the future > when the interface has proven stable for a reasonably long period. This is a good idea. I'll add the note. > Regarding the comment. > +// TODO - this needs effort to separate kernel and GUI > This issue should be resolved before the patch is committed to cvs. Is > it possible to use a SYNC dcop method to show a Yes/No messagebox and > return the result? Eventually we might be able to architect KMail so > that only ASYNC dcop methods are used, but I think it makes sense to > have a transitional period where SYNC methods are used. I've already thought about this issue, and I agree with you. It can get solved using a sync dcop call right now, but I also thought about a way to avoid such sync calls later. But this requires deeper changes in the existing codebase, I would like to concentrate on the dcop issue for now. I'll commit as soon as I have the TODOs eliminated. > At some point the various methods in KMailIface need to be deprecated > and replaced with equivalent/similar methods that take a > KMailClientIface parameter. (So that clients other than KMail, such as > KAddressBook/KOrganizer can take advantage of this work, and do so > simultaneously). > > Ok now onto the main issue with the patch, KMBroadcastStatus is used > in client.cpp. I think that's a cool idea but I want to be sure that > the implications of this decision are understood. > > Code can only exist in client space (all of client.cpp is in client > space) or in server space (kmkernel.cpp is in server space) but > (except in exceptional cases) not in both. > > As a general rule any code that deals with (Q)widgets belongs in > client space and everything else belongs in server space. > > KMBroadCastStatus deals with the status bar and little progress widget > so let's say that KMBroadcastStatus belongs in client space. Then all > usage of KMBroadcastStatus in server space has to be replace by calls > to the current KMailClientIface. (Does the existence of > slotSetStatusMsg, and slotSetStatusProgressPercent in KMailClientIface > show that you already understand this?) Yes, this is what I had in mind when I designed the client interface. > Examples of server space where such modifications are necessary > include imapaccountbase.cpp, kmaccount.cpp kmacctcachedimap.cpp, > kmacctexppop.cpp, kmacctimap.cpp, kmacctlocal.cpp, kmacctmaildir.cpp, > kmacctmgr.cpp, and kmsender.cpp. > > Examples of client space where such modifications are not necessary > include kmail_part.cpp, kmheaders.cpp, kmmainwidget.cpp, > kmmainwin.cpp. But these files contain classes with the opposite > requirement, all usage of server space must eventually be replaced > with usage of KMKernelIface. > > Once (in happy future space) this is done then it should be possible > to put all of the client space files into a separate library that > only accesses the rest of KMail through KMKernelIface. Then KMail > standalone will become a small program that links to this separate > client library and in main() starts up the KMail Kernel/Server if it > is not already running, and calls the KMKernelIface method that > creates a KMMainWindow. > > Other programs like KAddressBook will be able to link to this separate > KMail client library, and do things like open a composer window in the > KAddressBook process itself. > > Oh one other thing, (before the client KMail library can be made) all > the methods in kmailIface.h have to be deprecated and replaced by > methods that take a KMailClientIface parameter. > > And probably KMKernel should contain a KMailClientIface member > variable and accessor method as there can be only one client using > the kernel/server at any single time. > > Such an accessor method would make it easier to migrate server space > code from using client space code to using the KMailClientIface. Agree. However, I would like to go without the additional client parameters in the beginning. I prefer a second cycle to introduce this parameters. The reason is that I would like to get more feedback while I extend the dcop way file by file. Perhaps some other good idea will show up. > Did that make any sense? > > Don. > > On Saturday 05 July 2003 07:13, Andreas Gungl wrote: > > TIA, > > Andreas > > > > On Friday 04 July 2003 06:18, you (Don Sanders) wrote: > > > Hi Andreas, > > > > > > I just got back from a business trip. I'll try to reply to your > > > mail but it's still going to be a few more days before I've > > > cleared my work queue and have time. > > > > > > Don. > > > > > > On Sunday 29 June 2003 06:50, Andreas Gungl wrote: > > > > Hi, > > > > > > > > as nobody commented on my example even though I was away for > > > > one week, I want to ask you directly. > > > > > > > > Do you think, this is the way to go? Should I try to cover as > > > > much core classes as possible before a commit or is it better > > > > to commit early to give an example for all who want to join the > > > > work? Are the file names and class names okay from your point > > > > of view? > > > > > > > > I know, many questions. But I would like to have a solid basis > > > > right from the beginning instead of a quick hack which has no > > > > support from the core developers. Otherwise the time to work on > > > > this issue is not worth it for me. > > > > > > > > Regards, > > > > Andreas > > _______________________________________________ > KMail Developers mailing list > kmail@mail.kde.org > http://mail.kde.org/mailman/listinfo/kmail - -- ~ ' v ' // \\ /( )\ Powered by Penguin. ^ ' ^ -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.2.2-rc1-SuSE (GNU/Linux) iD8DBQE/DwJlVhjiFd4beU8RAgcLAJ4o3yEWFrX07d9Ib9/O0Ig3YPb28ACgxblk I9+OOzbNFYHgEP4lrG6K9aE= =wSQo -----END PGP SIGNATURE----- _______________________________________________ KMail Developers mailing list kmail@mail.kde.org http://mail.kde.org/mailman/listinfo/kmail