Review Request 120029: introduce the concept of package fallback

[David Edmundson]

> On Sept. 1, 2014, 4:53 p.m., David Edmundson wrote:
> > I need some the concept explaining, why would a developer set a fallbackpackage?
> > Is it not always org.kde.breeze.desktop?
> 
> Marco Martin wrote:
>     It would usually be the packagestructure setting it, in initPackage(), so setFallbackPackage follows the same pattern as addFileDefinition(), setRequired() and the likes.
>     
>     so, for the packages of type Plasma/LookAndFeel it would be org.kde.breeze.desktop
>     for packages of types Plasma/Shell, it would be org.kde.desktop
>     
>     even tough at some point it will probably have to be something more complicated, like the default lookandfeel one depending from  the current shell, if ever needed, will be feasible.

Ah, makes sense. Thanks.

Do you really expect a linked link big enough to cycle? I can't really see it going 3 deep. The cycle check is call cool though :)

Could you add a warning in setFallbackPackage() if it finds a cycle and no-ops. Could be confusing to debug otherwise.


> On Sept. 1, 2014, 4:53 p.m., David Edmundson wrote:
> > autotests/fallbackpackagetest.cpp, line 52
> > <https://git.reviewboard.kde.org/r/120029/diff/1/?file=309047#file309047line52>
> >
> >     This seems the variable names are backwards:
> >     
> >     the m_fallbackpackage is given a fallbackpackage of m_package
> 
> Marco Martin wrote:
>     as "package that has fallback" but yeah i can flip the names if more clear, since the api calls it the other way around

yes please. 
Or call it m_packageWithFallback


> On Sept. 1, 2014, 4:53 p.m., David Edmundson wrote:
> > src/plasma/package.cpp, line 925
> > <https://git.reviewboard.kde.org/r/120029/diff/1/?file=309050#file309050line925>
> >
> >     I dont' get how this const compiles, it clearly isn't.
> 
> Marco Martin wrote:
>     yeah, oddly does compiles fine.
>     may have to get even if ugly a const_cast instead there

I always get

const Type * 
and 
Type const *

confused.

This does make sense, it's the plasma::package thats const, not the pointer fastPackage


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120029/#review65648
-----------------------------------------------------------


On Sept. 1, 2014, 5:24 p.m., Marco Martin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120029/
> -----------------------------------------------------------
> 
> (Updated Sept. 1, 2014, 5:24 p.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> -------
> 
> introduces the concept of fallback for packages.
> this will replace the private statically linked class LookAndFeelAccess in workspace and desktop, but be more generic so will be usable for things like the shell package as well.
> The feature has an autotest as well to check it's actually working and doesn't break other stuff.
> 
> the package structures that will use this, will just set a package in their structure::initPackage()
> tough for the user of Package in c++ is possible as well to set the fallback package outside the structure if custm things are neede (it's guarded that cycles don't occur in the fallback chain)
> 
> 
> Diffs
> -----
> 
>   autotests/CMakeLists.txt 4e64f38 
>   autotests/data/testfallbackpackage/contents/ui/main.qml PRE-CREATION 
>   autotests/data/testfallbackpackage/metadata.desktop PRE-CREATION 
>   autotests/data/testpackage/contents/ui/otherfile.qml PRE-CREATION 
>   autotests/fallbackpackagetest.h PRE-CREATION 
>   autotests/fallbackpackagetest.cpp PRE-CREATION 
>   src/plasma/data/servicetypes/plasma-shell.desktop e2c83ba 
>   src/plasma/package.h 2c686d7 
>   src/plasma/package.cpp 6ad3321 
>   src/plasma/private/package_p.h d902eb1 
> 
> Diff: https://git.reviewboard.kde.org/r/120029/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20140901/4690d87e/attachment.html>