Review request for KSecretsService components
Albert Astals Cid
aacid at kde.org
Thu Nov 10 00:29:07 GMT 2011
A Dijous, 10 de novembre de 2011, Valentin Rusu vàreu escriure:
> On 11/10/2011 12:48 AM, Albert Astals Cid wrote:
> > A Dijous, 10 de novembre de 2011, Valentin Rusu vàreu escriure:
> >> Hello Albert,
> >> Thanks for the thourough review.
> >> On 11/09/2011 03:26 PM, Albert Astals Cid wrote:
> >>> This is scary, last time i used kwallet, i had to add a single line,
> >>> and now there are like a billion of classes?
> >> Welcome to the asynchronous jobs world. I had the same opinion at
> >> start.
> >> And, to be honest, that's why it takes so long to implement, as I'm
> >> not
> >> working on it only during my spare time.
> > Are you saying you don't have anything as simple as this in the new API?
> > QString walletName = KWallet::Wallet::NetworkWallet();
> > KWallet::Wallet *wallet = KWallet::Wallet::openWallet(walletName,
> > parentwid); if ( !wallet->hasFolder( "KPdf" ) )
> > wallet->createFolder( "KPdf" );
> > wallet->setFolder( "KPdf" );
> > wallet->readPassword( walletKey, retrievedPass )
> Yes, your code isn't asynchrounous ! (boo)
Yeah well, kparts gives you
virtual bool openUrl( const KUrl &url );
With that signature you are forced to either use synchronous code or create a
nested event loop, pick your poison.
> Well, I don't fully agree with this point of view, but in Randa they
> managed to convince me about the benefits.
> Take a look inside kwallet.cpp class to see how this new API should be used.
> Another example is the ksecrets API inside kdereview/ksecrets repository.
This answer makes me really scared in that it is not going to be 5 lines :-/
Ok, now i've looked at the API and I am not sure i like it, particulary
nonQtlike seems the way of searching things by passing a map with a key/value
structure of what you want to find.
I can see that it is very flexible, but string based api end up with people
writing "key" instead of "Key" and they lose days debugging why it fails.
Also i'm a bit confused about the docu of
SearchCollectionItemsJob * searchItems( const StringStringMap &attributes );
* KSecretsService uses overall the following standard attributes:
* "Label" : item's or collection's label
so i understand that "Label" is the only key of the map that you can use, and
then the example says
* StrinStringMap attrs;
* attrs["Key"] = "";
and uses "Key" ? Does this mean "Key" is a valid attribute too and should be
documented? Or should it say "Label"? Or i totally misunderstood the
Also note it says StrinStringMap, missing g.
More information about the kde-core-devel