KDE/kdebase/workspace/plasma/containments/desktop

[Aaron J. Seigo]

On Saturday 05 January 2008, Jason Stubbs wrote:
> On Sunday 06 January 2008 04:39:28 JST, Aaron J. Seigo wrote:
> > On Saturday 05 January 2008, Jason Stubbs wrote:
> > > SVN commit 757546 by jstubbs:
> > >
> > > Set a default wallpaper after constraints have been set so that
> > > geometry can be used to find the correct sized wallpaper. This also
> > > prevents updateBackground() from being called twice during startup.
> > >
> > > +    m_wallpaperPath = cg.readEntry("wallpaper", "");
> >
> > minor nit: QString() should be favoured over "" in code like this.
>
> To be explicit about the type wanted? Right, noted.

yes; it's also cheaper and faster as it doesn't allocate the "" internally ... 

> > > +        // Only set the wallpaper if constraints have been loaded
> > > +        if (screen() != -1) {
> > > +            updateBackground();
> > > +        }
> >
> > this will break for desktop containments that are not assigned to a
> > screen (e.g. alternate containments). it's an incorrect assumption that
> > all desktop containments will have a screen associated with them and will
> > result in such containments not repainting their background when they
> > should.
>
> This is an oversight on my part. Is there any such uses now that I can
> check behaviour with in the future?

you can always create another containment in the plasma-appletsrc (i usually 
have one kicking around in there)

> > in any case, perhaps a better fix might be add a parameter to
> > reloadConfig that tells it whether or not to also call updateBackground.
> > this would be set to false or whatever when called from init() but true
> > when called elsewhere.
>
> This works better and is clearer too. Patch attached which handles the
> slideshow case too.

looks good =)

> > p.s. this is really on the cusp or even beyond the cusp of the size of
> > patches that should be peer reviewed; i assume you had this peer reviewed
> > by someone?
>
> Not peer-reviewed, no... The metric I've been using is something to the
> effect of "does it touch more than one class? does it change any
> interfaces?" Will adding "does it add/remove code paths?" to that for 4.0.x
> changes be enough?

yeah; essentially things that are pretty obvious fixes (one liners, obvious 
crashers, etc) ... even then some things will fall through the cracks (which 
is why i read through the commit log msgs =) but should catch most 
review-worthy cases.

-- 
Aaron J. Seigo
humru othro a kohnu se
GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA  EE75 D6B7 2EB1 A7F1 DB43

KDE core developer sponsored by Trolltech
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : http://mail.kde.org/pipermail/panel-devel/attachments/20080106/0e0e9334/attachment-0001.pgp