On Monday, 2013-07-08, Pali Rohár wrote: > On Monday 08 July 2013 16:37:31 Kevin Krammer wrote: > > On Monday, 2013-07-08, Pali Rohár wrote: > > > On Thursday 04 July 2013 18:38:40 Kevin Krammer wrote: > > > > Hi, > > > > > > > > I had a look at the current state of the integration > > > > clone. > > > > > > > > TrojitaPluginJob.h > > > > > > > > - autoDelete() and isRunning() should be const and normal > > > > public method (not slots). > > > > > > Ok, changed. > > > > const > > > > You could keep setAutoDelete a slot, I was only referring to > > the two getter methods :) > > It is really needed?
Up to the Trojita developers I guess, since I don't know about coding style in this code base. In KDE PIM getters are const so that's why I mentioned it. > > I would advise against using unstranslatable strings in the > > user interface. A better approach is to have an identifier > > (used internally) and a name (displayed to the user). > > And what to show when user choose plugin A and then plugin A will > be deleted from system? In config file is store only plugin name If you want to display the plugin that has become unavailable you could additionally store the name, no? The main problem with using the identifier as a user visible string is that it is a foreign collection of symbols for anyone with non-latin script. > > > > === > > > > > > > > KDE AddressBook > > > > > > > > - using deprecated KABC API parts, e.g. KABC::AddressBook. > > > > Should use Akonadi API instead > > > > > > KABC::AddressBook still working fine (on KDE4.10) and is > > > faster on my notebook than akonadi. I do not want hard > > > dependency in Trojita on akonadi which slow down > > > notebook-like devices. > > > > Well, the plugin is called "KDE", suggesting its purpose is > > KDE integration :) This is also the topic of the GSoC > > project. > As I wrote in previous mail. I can create another KDE addressbook > plugin which will use akonadi (and not kabc). If you want to have a vcard addressbook plugin then by all means. If you want to use KDE's VCard library, again, by all means. StdAddressBook is quite some overhead for just working with a vcard file. Anyway, I was mostly commenting on this because the plugin in the "KDE" directory, which I assume is for KDE integration. > I'm not using akonadi, but traditional kabc kresource (not > kresource <--> akonadi compatibility plugin!). So I will use this > non-akonadi KABC::AddressBook plugin in Trojita. Sure, there is no need to use Akonadi for a vcard addressbook. I was talking about the KDE addressbook integration :) Additonally, as I wrote above, using KABC::AddressBook is, first, an unnecessary overhead for handling a simple vcard file and, second, at some point just a compatibility API on top of Akonadi. > private: > QHash<Job *, QEventLoop *> m_loops; > QHash<Job *, result> m_results; > > New Password/Addressbook job with associated eventloop is > inserted to m_loops. When job finish it will call slot. Slot will > insert pair <job, result> to m_results, remove eventloop from > m_loops and stop it. Then continue execution and result will be > removed from m_results. Job, eventloop and this (ptr) are stored > in QPointer for checking if somebody did not deleted them. Sounds better, but I wonder if it doesn't get into a similar effort as doing it properly asynchronous. Cheers, Kevin -- Kevin Krammer, KDE developer, xdg-utils developer KDE user support, developer mentoring
signature.asc
Description: This is a digitally signed message part.
