On Saturday, 2013-06-29, Pali Rohár wrote: > > - Not sure whether you need to #include <QPointer>; perhaps a > > forward declaration is enough (haven't checked, might be > > wrong) > > This will not work, because in class is full QPointer object - > not pointer or reference.
Could still be possible to forward declare, I think you would only need to add a non-inline destructor to the class that has the QPointer member. > > - While common in the existing desktop software, I don't find > > "Default Trojita Addressbook" to be meaningful description. > > As a user, I'd like to know that it's accessing an > > abook-formatted file which can be shared with mutt, not that > > it happens to be used by trojita by default. > > Ok. Also name() and description() should probably be marked for translation. Some other suggestions: - Since you are discarding the error value, why not just have one signal that is emitted regardless of success or error? - the start() methods of synchronous jobs should probably not emit signals, better use QMetaObject::invokeMethod() or a QTimer on a private slots to trigger the actual job code. This way start() will always return before any result is delivered, making their behavior more like the ones really asynchronous jobs will have. - Never ever delete an object in a slot called by the object's signals. Always use deleteLater() - AbookAddressbook has lots of public API that is not in AddressbookInterface (e.g. model). Cheers, Kevin -- Kevin Krammer, KDE developer, xdg-utils developer KDE user support, developer mentoring
signature.asc
Description: This is a digitally signed message part.
