Remote Widget API Review - The Revenge

[Kevin Ottens]

Hello,

Plasma's API police on the line! (even if remotely)

On Tuesday 1 September 2009 20:46:37 Rob Scheepmaker wrote:
> Here's the remote widget api as it is after review at Tokamak.... still
> something wrong? Speak now, or forever hold your peace.

OK, not much (potential) issues left from my POV, most of the bigger concerns 
I had got addressed, good job guys!

Still there's a few point I don't get or which concern me. Here they are:

 * About Access*Applet*Job and the "remote applet" naming in AccessManager, 
isn't it somewhat inconsistent with the fact that later on we can publish 
Applets, Services, DataEngines and Packages? I think the "resource" naming 
instead of "applet" naming being more generic would work better there.

 * In AuthorizationInterface::authorizationRequest(), I have this feeling that 
AuthorizationRule is being abused (assuming it's main role is to express a 
rule) to express a one time request (similar to ClientPinRequest). Perhaps 
there should be an AuthorizationRequest class which you can accept() or 
reject(), and which would point to the AuthorizationRule it's referring to?

 * Why are ClientPinRequest and AuthorizationRule inheriting from QObject? I 
feel that they should be made value based, using QSharedDataPointer since 
they're mainly data stores.

 * AccessAppletJob only gives access to an Applet instance, what about    
DataEngines, etc.? I really fail to see how to access remote Stuff (for Stuff 
!= Applet) while it's crystal clear how to access applets or to publish Stuff.

 * TrustLevel should probably be CredentialLevel?

 * I'm not sold on the name Identity... too easy to confuse with some PIM 
stuff IMHO (although I might be biased here). I'd lean toward something more 
technical for this one CallerIdentifier? ChannelIdentifier? ClientKey (since 
it has some crypto feature)? ClientCertificate? (hmm... the latest one as my 
preference, it's definitely about certification).

 * toPublicIdentity => strippedIdentity? or even stripped? (thinking 
QString::simplified() or QString::trimmed() here, while it's about "stripping" 
private data from the object)

 * Shouldn't PackageMetaData remoteLocation type be KUrl? (somehow I'd expect 
it to be used in cunjunction with AccessAppletJob which uses KUrl)

 * I guess it's a copy/paste mistake or such, there's one occurrence of 
PublicationMethods, did you mean AnnouncementMethods?

 * The definition of AnnouncementMethods is missing from what you send so I 
can't evaluate it.

 * unpublish() on Service takes a name parameter, seems fine (since publish 
has one as well), but what is the name parameter for in the unpublish() method 
of Package and Applet?

OK, this list is slightly longer that I first expected, sorry about that...

Regards.
-- 
Kévin 'ervin' Ottens, http://ervin.ipsquad.net
"Ni le maître sans disciple, Ni le disciple sans maître,
Ne font reculer l'ignorance."
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part.
Url : http://mail.kde.org/pipermail/plasma-devel/attachments/20090901/eb158af9/attachment.sig