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. > > Thanks, > > 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 kwalletmanager? My comments on the kdelibs API: - ksecretsservice/ksecretsservicecodec.h * 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 - ksecretsservice/ksecretsservicecollection.h * 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 - ksecretsservice/ksecretsservicedbustypes.h Contains a plain struct and some inline functions, what if those have to change? - ksecretsservice/ksecretsserviceitem.h * Same question about ownership of jobs - ksecretsservice/ksecretsserviceitemjobs.h * 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 myself? * SecretItemJob does not have a d-pointer * void finished( ItemJobError, const QString& msg =""); should probably be void finished( ItemJobError, const QString& msg = QString()); - ksecretsservice/ksecretsservicesecret.h Is installed and has no documentation Albert > > > > 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)