Review request for KSecretsService components
kde at rusu.info
Thu Nov 10 00:00:05 GMT 2011
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)
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.
>>> * createItem falls in the "let's use a bool instead of an enum
>>> because it>
>>> just have two values" trap for the replace parameter
>> Well, I don't agree. When presenting a new item to the collection, you
>> should specify to replace existing item or not. Yes/No is boolean for me.
> That is the problem, that a boolean is Yes/No, and then you end up with a call
> that says
> foo->createItem(label, attributes, mySecret, false);
> And it seems you do not want to create the item :D while
> foo->createItem(label, attributes, mySecret, DoNotReplace);
> is much more obvious.
> More on my side at
Ok, I see. I'll change that.
> Of course that is a minor thing and won't complain too much if you decide to
> disagree with me ;-)
Valentin Rusu (IRC valir, KDE vrusu)
KSecretsService (former KSecretService, KWallet replacement)
More information about the kde-core-devel