Le Tuesday 12 August 2008, Aaron J. Seigo a écrit : > On Monday 11 August 2008, Kevin Ottens wrote: > > Hello, > > > > Same dumb method as usual from me. > > > > Le Tuesday 05 August 2008, Aaron J. Seigo a écrit : > > > the affected classes are Containment, with a setter and getter for the > > > wallpaper > > > > I'd probably add a setWallpaper(Wallpaper*) so that the setter/getter > > couple becomes symetric with an extra convenience version (the one doing > > the load for you). > > ownership of the Wallpaper* would then be taken by the Containment in this > case;
Is that required? > there are still ways to screw it up at this point. i'm always wary of > these kinds of setters for this reason. Yeah, indeed, I'm always pondering about silently transfering ownership. So if this transfer is required I might live with the non symmetric getter/setter pair. If it's not we'd probably have to add a parent argument in the load method. > > > and the new class Wallpaper. > > > > A few comments: > > * I think it's worth it to have the "icon" property too > > +1 > > > * I don't get what the "mode" thing is about. Any more specific use > > case? I admit I had to look for the apidox for this one. I found a small > > blurb in the class apidox about modes, but from the relevant methods > > there's nothing about what the modes are. Last, the class apidox didn't > > make me figure out what they are exactly (switch to using an "Action" > > term at some point). > > yes, this needs to be made clearer. the concept is that one plugin may > provide multiple wallpaper methods. > > our current wallpaper renderer does Single Image, Slideshow and Plain > Color. so, three different modes. Gotcha. What about RenderingMode? Just "mode" doesn't convey enough information to me. > > * Depending on what we figure out regarding what the modes are, it might > > be worth it to have a small "WallpaperMode" class (it holds already three > > properties, and could grow). > > which 3 properties? /me looks at his hand again. OK, I can't count it seems, the 2 properties so that obviously less of a problem *for now*. Could that grow? I've a hard time figuring out if it's likely or not... But somehow I clearly imagine that modes could have more information attached to them, for instance "is it animated?", for decision taking, for instance should I fall back to a simpler mode to save power or bandwidth. > > * I don't get the action parameter from the init() method (but since I > > didn't understand th mode thing, that's probably related). > > that's actually supposed to be mode, not action. OK, thanks. 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