On Wednesday, 2013-06-26, Jan Kundrát wrote: > On Friday, 21 June 2013 16:05:14 CEST, Pali Rohár wrote: > > I'm sending new password interface. Now instead of type enum, > > plugin should emit one of result signal (with this pointer). > > Also I > > removed private variables which are not needed anymore. > > Thanks. > > A couple comments: > > - I am sceptical about the usefullness of the "PasswordJob *job" in the > job's signals, but I defer to Kevin's experience when he says that these > are useful in practice. OK.
Well, it is useful since you get the job itself into the slot and don't have to use QObject::sender() and a cast but that is mostly a matter of taste I guess. What I would do different is what you already suggested in a different mail, i.e. using sub classes for each type of job, each job only needing the API for its task, no need for things like "should only be called by ... jobs". But again mostly a matter of taste, I am just very used to KDE jobs where all are (directly or indirectly) based on KJob. > - I dislike the inlined protected constructor. Why do you need it? I guess that the idea is that each plugin has to create its own job implementation anyway, i.e. Password job is never instantiated directly. But it should probably not be inline. Cheers, Kevin -- Kevin Krammer, KDE developer, xdg-utils developer KDE user support, developer mentoring
signature.asc
Description: This is a digitally signed message part.
