Minutes Monday Plasma hangout

David Edmundson david at davidedmundson.co.uk
Mon Feb 3 17:07:51 UTC 2014


> sebas:
> - sebas/themeswitch branch allows switching themes, almost good to merge:
>         - Blur should depend on theme values
>         - styled icons don't change on theme changes (notmart will have a look)
>

I did a code review of the branch.
A few nitpicks

Missing & in src/plasma/package.cpp
+ foreach (const QString pack, entries) {

---------------------------

1fedbb587c7a94674484df0fe57d24299d000a06

avoid committing commented out code (the qDebugs)

---------------------------

+ if (!themeName.isEmpty()) {

+ Plasma::Theme *t = new Plasma::Theme(this);

+ t->setThemeName(themeName);

+ }

This API sucks a bit. (not the fault of your branch)

Changing a member of an object, should not affect lots of instances of
that object.

Can I make this a static method in Plasma::Theme instead?

Whilst I'm here:
 - setUseGlobalSettings does nothing. It unsets the theme name.. but
then if you call setThemeName you'll then change things globally as
it's still the same d pointer.

 - why do we do our own ref counting? QHash<QString,
QSharedPtr<ThemePrivate>> themes; would make all code simpler.

David


More information about the Plasma-devel mailing list