Remote Widget API Review - The Revenge
ervin at kde.org
Tue Sep 1 21:36:53 CEST 2009
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...
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
Size: 197 bytes
Desc: This is a digitally signed message part.
Url : http://mail.kde.org/pipermail/plasma-devel/attachments/20090901/eb158af9/attachment.sig
More information about the Plasma-devel