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."
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel