Review request for KSecretsService components
Albert Astals Cid
aacid at kde.org
Wed Nov 9 14:26:12 GMT 2011
A Dimarts, 8 de novembre de 2011, Valentin Rusu vàreu escriure:
> Hello Again,
> The freeze will come in less thant two days now and I'd like to know if
> anyone reviewed these components.
> On 10/31/2011 11:48 PM, Valentin Rusu wrote:
> > Hello,
> > Please be advised three repostories need review before integration
> > into the next release:
> > 1. /kdereview/ksecretservice
> > 2. kdelibs branch ksecretsservice
> > 3. /kdereview/ksecrets
> > *1. /kdereview/ksecretservice*
> > The first one has several subdirectories. The only relevant one is the
> > daemon subdirectory. Other directories contents was already moved to
> > other repositories (see below).
> > This daemon directory contains the sources of the future
> > kde-runtime/ksecretsserviced
> > It needs testing but it's quite usable.
> > *2. kdelibs branch ksecretsservice*
> > kdelibs/kdeui/ksecretsservice
> > This is the API KDE applications will be supposed to use instead of
> > KWallet class. Tools listed below already use this API.
This is scary, last time i used kwallet, i had to add a single line, and now
there are like a billion of classes? Or is this API for more complex apps like
My comments on the kdelibs API:
* Seems to be installed but has no documentation at all
* Uses references for output parameters when KDE/Qt usually uses pointers
* Does not use d-pointers thus maintaining ABI is going to be difficult
* I miss a note saying who belongs the ownsership of all those job that come
as result of the getters. Do I have to delete them?
* createItem falls in the "let's use a bool instead of an enum because it
just have two values" trap for the replace parameter
* There are a few of spacing "glitches", e.g.
const QVariantMap collectionProperties = QVariantMap(),
const WId &promptParentWindowId =0 );
* There are a few not implemented signals
Contains a plain struct and some inline functions, what if those have to
* Same question about ownership of jobs
* Without having at all a clue on what it is used for, i miss a note saying
to who belongs the ownership of the SecretItem passed to the constructor
and returned in the secretItem() function, i.e. do i have to delete it
* SecretItemJob does not have a d-pointer
* void finished( ItemJobError, const QString& msg =""); should probably be
void finished( ItemJobError, const QString& msg = QString());
Is installed and has no documentation
> > kdelibs/kdeui/util/kwallet.cpp
> > Contains code depending on a configuration flag that directs calls to
> > ksecretsserviced instead of kwalletd, via the new API.
> > *3. /kdereview/ksecrets*
> > Contains several tools in a less or more mature state:
> > a. kwl2kss - tool to import kwallet files to ksecretsservice,
> > b. ksecrets - tool to list the contents of a ksecretsservice
> > collection (e.g. wallet),
> > c. kio - KIO slave in a just started state, intended to show
> > collections in konqueror or dolphin,
> > d. secretsync - this was the tool I initially wanted to do for
> > KWallet, but drought me into ksecretsservice :-)
> > It's half way implemented.
> > The mandatory components for next release would be 1, 2, 3 (a, b), the
> > others may wait, but releasing them may cause no harm if communication
> > is done right (I'll take care of that).
> > Thanks for your comments (any comments),
> Valentin Rusu (IRC valir, KDE vrusu)
> KSecretsService (former KSecretService, KWallet replacement)
More information about the kde-core-devel