Re: Review Request 126185: Make the KAppTemplate CMake module global

[Alex Merry]

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

Ship it!


OK, just one line break in the docs, then it can go in.


kde-modules/KDEPackageAppTemplates.cmake (line 19)
<https://git.reviewboard.kde.org/r/126185/#comment61779>

New paragraph, please.


- Alex Merry


On Dec. 28, 2015, 9:37 a.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126185/
> ---
> 
> (Updated Dec. 28, 2015, 9:37 a.m.)
> 
> 
> Review request for Build System, KDE Frameworks, Plasma, Aleix Pol Gonzalez, 
> and Simon Wächter.
> 
> 
> Repository: extra-cmake-modules
> 
> 
> Description
> ---
> 
> templates are very useful as teaching tool in order to make
> a minimal application that uses a certain framework.
> templates in the KAppTemplate repository will always get forgotten
> (plus kapptemplate is not really necessary as they work in kdevelop as well)
> An ideal situation would be frameworks having templates in their own repos
> with templates of barebone apps using the main framework features.
> In order to do that, the cmake stuff needed in order to correctly install
> a template needs to be ported to a place avaiable to all frameworks
> 
> 
> Diffs
> -
> 
>   docs/kde-module/KDEPackageAppTemplates.rst PRE-CREATION 
>   kde-modules/KDEInstallDirs.cmake b7cd34d 
>   kde-modules/KDEPackageAppTemplates.cmake PRE-CREATION 
>   tests/CMakeLists.txt 1a66f56 
>   tests/KDEPackageAppTemplatesTest/CMakeLists.txt PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/check.cmake.in PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/CMakeLists.txt PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/Messages.sh PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/README PRE-CREATION 
>   
> tests/KDEPackageAppTemplatesTest/qml-plasmoid/package/contents/images/pairs.svgz
>  PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/package/contents/ui/main.qml 
> PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/package/metadata.desktop 
> PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/qml-plasmoid.kdevtemplate 
> PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/qml-plasmoid.png PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126185/diff/
> 
> 
> Testing
> ---
> 
> done some templates installed by plasma-framework
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 126185: Make the KAppTemplate CMake module global

[Alex Merry]

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


Sorry for the delay; Christmas happened :-)

Just two more small (but important) things, and then we're ready to go!


kde-modules/KDEPackageAppTemplates.cmake (line 1)
<https://git.reviewboard.kde.org/r/126185/#comment61735>

This documentation won't be added to the generated docs unless you put a 
link file in the `docs/kde-module` directory. It's pretty straightforward - 
just copy and edit one of the other files in that directory.



kde-modules/KDEPackageAppTemplates.cmake (line 16)
<https://git.reviewboard.kde.org/r/126185/#comment61734>

I would phrase this as something like

INSTALL_DIR is the directory to install the template package to. In 
most cases you will want to use the variable KDE_INSTALL_KTEMPLATESDIR from 
:kde-module:`KDEInstallDirs`.

And then put a blank line before the bit about TEMPLATES, so they're 
separate paragraphs.


- Alex Merry


On Dec. 18, 2015, 10:55 a.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126185/
> ---
> 
> (Updated Dec. 18, 2015, 10:55 a.m.)
> 
> 
> Review request for Build System, KDE Frameworks, Plasma, Aleix Pol Gonzalez, 
> and Simon Wächter.
> 
> 
> Repository: extra-cmake-modules
> 
> 
> Description
> ---
> 
> templates are very useful as teaching tool in order to make
> a minimal application that uses a certain framework.
> templates in the KAppTemplate repository will always get forgotten
> (plus kapptemplate is not really necessary as they work in kdevelop as well)
> An ideal situation would be frameworks having templates in their own repos
> with templates of barebone apps using the main framework features.
> In order to do that, the cmake stuff needed in order to correctly install
> a template needs to be ported to a place avaiable to all frameworks
> 
> 
> Diffs
> -
> 
>   kde-modules/KDEInstallDirs.cmake b7cd34d 
>   kde-modules/KDEPackageAppTemplates.cmake PRE-CREATION 
>   tests/CMakeLists.txt 1a66f56 
>   tests/KDEPackageAppTemplatesTest/CMakeLists.txt PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/check.cmake.in PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/CMakeLists.txt PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/Messages.sh PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/README PRE-CREATION 
>   
> tests/KDEPackageAppTemplatesTest/qml-plasmoid/package/contents/images/pairs.svgz
>  PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/package/contents/ui/main.qml 
> PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/package/metadata.desktop 
> PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/qml-plasmoid.kdevtemplate 
> PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/qml-plasmoid.png PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126185/diff/
> 
> 
> Testing
> ---
> 
> done some templates installed by plasma-framework
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 126185: Make the KAppTemplate CMake module global

[Alex Merry]

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



tests/CMakeLists.txt (line 43)
<https://git.reviewboard.kde.org/r/126185/#comment61417>

Use the add_test_macro command, rather than add_subdirectory. This will 
treat the subdirectory as an indepentend CMake project - running CMake on it, 
and make, and (optionally) make install.

Just look further down the file at lines 124-131 for an example.



tests/KDEPackageAppTemplatesTest/run_test.cmake.config (lines 1 - 11)
<https://git.reviewboard.kde.org/r/126185/#comment61416>

This should go in the CMakeLists.txt file, in place of the add_test command.

All your run_test.cmake script needs to do is use execute_process() to 
untar the installed tarball, and examine the contents. I highly recommend you 
look at ECMInstallIconsTest.


- Alex Merry


On Dec. 17, 2015, 11:15 a.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126185/
> ---
> 
> (Updated Dec. 17, 2015, 11:15 a.m.)
> 
> 
> Review request for Build System, KDE Frameworks, Plasma, Aleix Pol Gonzalez, 
> and Simon Wächter.
> 
> 
> Repository: extra-cmake-modules
> 
> 
> Description
> ---
> 
> templates are very useful as teaching tool in order to make
> a minimal application that uses a certain framework.
> templates in the KAppTemplate repository will always get forgotten
> (plus kapptemplate is not really necessary as they work in kdevelop as well)
> An ideal situation would be frameworks having templates in their own repos
> with templates of barebone apps using the main framework features.
> In order to do that, the cmake stuff needed in order to correctly install
> a template needs to be ported to a place avaiable to all frameworks
> 
> 
> Diffs
> -
> 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/Messages.sh PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/CMakeLists.txt PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/CMakeLists.txt PRE-CREATION 
>   kde-modules/KDEPackageAppTemplates.cmake PRE-CREATION 
>   tests/CMakeLists.txt 1a66f56 
>   kde-modules/KDEInstallDirs.cmake b7cd34d 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/README PRE-CREATION 
>   
> tests/KDEPackageAppTemplatesTest/qml-plasmoid/package/contents/images/pairs.svgz
>  PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/package/contents/ui/main.qml 
> PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/package/metadata.desktop 
> PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/qml-plasmoid.kdevtemplate 
> PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/qml-plasmoid.png PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/run_test.cmake.config PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126185/diff/
> 
> 
> Testing
> ---
> 
> done some templates installed by plasma-framework
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 126185: Make the KAppTemplate CMake module global

[Alex Merry]

> On Dec. 12, 2015, 3:40 p.m., Alex Merry wrote:
> > Ooh, also, please write a unit test. I can help with that if you find the 
> > idea of writing a CMake-based unit test daunting, but you can look in the 
> > tests directory for inspiration.
> 
> Marco Martin wrote:
> I'm not sure what the test should do...
> perhaps having in the repo a tarball and a source template, make it 
> generate and then compare the files?
> 
> Alex Merry wrote:
> Yes, that would work. Or you could have a source template, and check the 
> installed file can be extracted and check the extracted contents. Don't 
> forget to make sure that files you think should be excluded are in fact 
> omitted.
> 
> Marco Martin wrote:
> if i try to use kde_add_app_templates from a run_test.cmake i get the 
> message KDETemplateGenerator.cmake:97 (add_custom_target):
>   Command add_custom_target() is not scriptable
>   any idea what it could be?
> 
> Alex Merry wrote:
> Ah, you'll need to do a project-style test. See ECMPoQmToolsTest, or 
> ECMInstallIconsTest. The idea is you create a CMakeLists.txt file as though 
> it were a project using this module, and make, say, a check_tree.cmake script 
> to check that the right things were installed in the right places. Lines 
> 124-131 of tests/CMakeLists.txt are what run ECMInstallIconsTest.
> 
> Marco Martin wrote:
> still not completely understanding.. do i have to execute the operations 
> i test in the test dirs cmakelist or have the run_test.cmake generate another 
> cmakelist and then  run another cmake on top of that?
> 
> Marco Martin wrote:
> in the latest version it successfully to just check for the existence of 
> the tarball.
> is it possible to uncompress it as well in the test script? (i get the 
> same non scriptable error if i try to call the cmake untar command)

That's because you need to use execute_process().

For the general structure, I really recommend you take ECMInstallIconsTest as 
your starting point.


- Alex


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


On Dec. 17, 2015, 11:15 a.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126185/
> ---
> 
> (Updated Dec. 17, 2015, 11:15 a.m.)
> 
> 
> Review request for Build System, KDE Frameworks, Plasma, Aleix Pol Gonzalez, 
> and Simon Wächter.
> 
> 
> Repository: extra-cmake-modules
> 
> 
> Description
> ---
> 
> templates are very useful as teaching tool in order to make
> a minimal application that uses a certain framework.
> templates in the KAppTemplate repository will always get forgotten
> (plus kapptemplate is not really necessary as they work in kdevelop as well)
> An ideal situation would be frameworks having templates in their own repos
> with templates of barebone apps using the main framework features.
> In order to do that, the cmake stuff needed in order to correctly install
> a template needs to be ported to a place avaiable to all frameworks
> 
> 
> Diffs
> -
> 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/Messages.sh PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/CMakeLists.txt PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/CMakeLists.txt PRE-CREATION 
>   kde-modules/KDEPackageAppTemplates.cmake PRE-CREATION 
>   tests/CMakeLists.txt 1a66f56 
>   kde-modules/KDEInstallDirs.cmake b7cd34d 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/README PRE-CREATION 
>   
> tests/KDEPackageAppTemplatesTest/qml-plasmoid/package/contents/images/pairs.svgz
>  PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/package/contents/ui/main.qml 
> PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/package/metadata.desktop 
> PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/qml-plasmoid.kdevtemplate 
> PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/qml-plasmoid.png PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/run_test.cmake.config PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126185/diff/
> 
> 
> Testing
> ---
> 
> done some templates installed by plasma-framework
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 126185: Make the KAppTemplate CMake module global

[Alex Merry]

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


I just want to say thanks for keeping at this - I feel kinda bad about the 
sheer number of changes I'm throwing at you! If you want to make me do some of 
the work, you can create a feature branch and point me at it, but I'm hoping 
this review is setting you up for an easy ride if you want to contribute 
something else to e-c-m in the future.

- Alex Merry


On Dec. 16, 2015, 12:25 p.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126185/
> ---
> 
> (Updated Dec. 16, 2015, 12:25 p.m.)
> 
> 
> Review request for Build System, KDE Frameworks, Plasma, Aleix Pol Gonzalez, 
> and Simon Wächter.
> 
> 
> Repository: extra-cmake-modules
> 
> 
> Description
> ---
> 
> templates are very useful as teaching tool in order to make
> a minimal application that uses a certain framework.
> templates in the KAppTemplate repository will always get forgotten
> (plus kapptemplate is not really necessary as they work in kdevelop as well)
> An ideal situation would be frameworks having templates in their own repos
> with templates of barebone apps using the main framework features.
> In order to do that, the cmake stuff needed in order to correctly install
> a template needs to be ported to a place avaiable to all frameworks
> 
> 
> Diffs
> -
> 
>   kde-modules/KDEInstallDirs.cmake b7cd34d 
>   kde-modules/KDEPackageAppTemplates.cmake PRE-CREATION 
>   tests/CMakeLists.txt 1a66f56 
>   tests/KDEPackageAppTemplatesTest/CMakeLists.txt PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/CMakeLists.txt PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/Messages.sh PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/README PRE-CREATION 
>   
> tests/KDEPackageAppTemplatesTest/qml-plasmoid/package/contents/images/pairs.svgz
>  PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/package/contents/ui/main.qml 
> PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/package/metadata.desktop 
> PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/qml-plasmoid.kdevtemplate 
> PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/qml-plasmoid.png PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/run_test.cmake.config PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126185/diff/
> 
> 
> Testing
> ---
> 
> done some templates installed by plasma-framework
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 126185: Make the KAppTemplate CMake module global

[Alex Merry]

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



tests/KDEPackageAppTemplatesTest/qml-plasmoid/package/contents/ui/main.qml 
(lines 1 - 18)
<https://git.reviewboard.kde.org/r/126185/#comment61372>

BSD code only in extra-cmake-modules, please. Frankly, your app template 
can be complete rubbish, as long as it's packaged up properly.


- Alex Merry


On Dec. 16, 2015, 12:25 p.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126185/
> ---
> 
> (Updated Dec. 16, 2015, 12:25 p.m.)
> 
> 
> Review request for Build System, KDE Frameworks, Plasma, Aleix Pol Gonzalez, 
> and Simon Wächter.
> 
> 
> Repository: extra-cmake-modules
> 
> 
> Description
> ---
> 
> templates are very useful as teaching tool in order to make
> a minimal application that uses a certain framework.
> templates in the KAppTemplate repository will always get forgotten
> (plus kapptemplate is not really necessary as they work in kdevelop as well)
> An ideal situation would be frameworks having templates in their own repos
> with templates of barebone apps using the main framework features.
> In order to do that, the cmake stuff needed in order to correctly install
> a template needs to be ported to a place avaiable to all frameworks
> 
> 
> Diffs
> -
> 
>   kde-modules/KDEInstallDirs.cmake b7cd34d 
>   kde-modules/KDEPackageAppTemplates.cmake PRE-CREATION 
>   tests/CMakeLists.txt 1a66f56 
>   tests/KDEPackageAppTemplatesTest/CMakeLists.txt PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/CMakeLists.txt PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/Messages.sh PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/README PRE-CREATION 
>   
> tests/KDEPackageAppTemplatesTest/qml-plasmoid/package/contents/images/pairs.svgz
>  PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/package/contents/ui/main.qml 
> PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/package/metadata.desktop 
> PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/qml-plasmoid.kdevtemplate 
> PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/qml-plasmoid.png PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/run_test.cmake.config PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126185/diff/
> 
> 
> Testing
> ---
> 
> done some templates installed by plasma-framework
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 126185: Make the KAppTemplate CMake module global

[Alex Merry]

On Dec. 16, 2015, 7:14 p.m., Marco Martin wrote:
> > You might want to generate the documentation and look at it (you need the 
> > Sphinx documentation tool installed, then you just build 
> > extra-cmake-modules, and look in docs/html in the build directory).

In particular, you'll need to add a link file in docs/kde-module.


- Alex


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


On Dec. 16, 2015, 12:25 p.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126185/
> ---
> 
> (Updated Dec. 16, 2015, 12:25 p.m.)
> 
> 
> Review request for Build System, KDE Frameworks, Plasma, Aleix Pol Gonzalez, 
> and Simon Wächter.
> 
> 
> Repository: extra-cmake-modules
> 
> 
> Description
> ---
> 
> templates are very useful as teaching tool in order to make
> a minimal application that uses a certain framework.
> templates in the KAppTemplate repository will always get forgotten
> (plus kapptemplate is not really necessary as they work in kdevelop as well)
> An ideal situation would be frameworks having templates in their own repos
> with templates of barebone apps using the main framework features.
> In order to do that, the cmake stuff needed in order to correctly install
> a template needs to be ported to a place avaiable to all frameworks
> 
> 
> Diffs
> -
> 
>   kde-modules/KDEInstallDirs.cmake b7cd34d 
>   kde-modules/KDEPackageAppTemplates.cmake PRE-CREATION 
>   tests/CMakeLists.txt 1a66f56 
>   tests/KDEPackageAppTemplatesTest/CMakeLists.txt PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/CMakeLists.txt PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/Messages.sh PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/README PRE-CREATION 
>   
> tests/KDEPackageAppTemplatesTest/qml-plasmoid/package/contents/images/pairs.svgz
>  PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/package/contents/ui/main.qml 
> PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/package/metadata.desktop 
> PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/qml-plasmoid.kdevtemplate 
> PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/qml-plasmoid.png PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/run_test.cmake.config PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126185/diff/
> 
> 
> Testing
> ---
> 
> done some templates installed by plasma-framework
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 126185: Make the KAppTemplate CMake module global

[Alex Merry]

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



kde-modules/KDEPackageAppTemplates.cmake (line 10)
<https://git.reviewboard.kde.org/r/126185/#comment61369>

Frameworks has a capital F.



kde-modules/KDEPackageAppTemplates.cmake (line 14)
<https://git.reviewboard.kde.org/r/126185/#comment61365>

Signature is still wrong.



kde-modules/KDEPackageAppTemplates.cmake (lines 16 - 37)
<https://git.reviewboard.kde.org/r/126185/#comment61368>

None of this should be indented.

Beware of how Sphinx formats your list of placeholders, though. Consider 
doing the list in the same way as KDEInstallDirs.cmake lists its variables.



kde-modules/KDEPackageAppTemplates.cmake (line 20)
<https://git.reviewboard.kde.org/r/126185/#comment61366>

"as if it was", instead of "as it was"?



kde-modules/KDEPackageAppTemplates.cmake (line 22)
<https://git.reviewboard.kde.org/r/126185/#comment61363>

missing closing parenthesis )?


You might want to generate the documentation and look at it (you need the 
Sphinx documentation tool installed, then you just build extra-cmake-modules, 
and look in docs/html in the build directory).

- Alex Merry


On Dec. 16, 2015, 12:25 p.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126185/
> ---
> 
> (Updated Dec. 16, 2015, 12:25 p.m.)
> 
> 
> Review request for Build System, KDE Frameworks, Plasma, Aleix Pol Gonzalez, 
> and Simon Wächter.
> 
> 
> Repository: extra-cmake-modules
> 
> 
> Description
> ---
> 
> templates are very useful as teaching tool in order to make
> a minimal application that uses a certain framework.
> templates in the KAppTemplate repository will always get forgotten
> (plus kapptemplate is not really necessary as they work in kdevelop as well)
> An ideal situation would be frameworks having templates in their own repos
> with templates of barebone apps using the main framework features.
> In order to do that, the cmake stuff needed in order to correctly install
> a template needs to be ported to a place avaiable to all frameworks
> 
> 
> Diffs
> -
> 
>   kde-modules/KDEInstallDirs.cmake b7cd34d 
>   kde-modules/KDEPackageAppTemplates.cmake PRE-CREATION 
>   tests/CMakeLists.txt 1a66f56 
>   tests/KDEPackageAppTemplatesTest/CMakeLists.txt PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/CMakeLists.txt PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/Messages.sh PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/README PRE-CREATION 
>   
> tests/KDEPackageAppTemplatesTest/qml-plasmoid/package/contents/images/pairs.svgz
>  PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/package/contents/ui/main.qml 
> PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/package/metadata.desktop 
> PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/qml-plasmoid.kdevtemplate 
> PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/qml-plasmoid.png PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/run_test.cmake.config PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126185/diff/
> 
> 
> Testing
> ---
> 
> done some templates installed by plasma-framework
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 126185: Make the KAppTemplate CMake module global

[Alex Merry]

> On Dec. 12, 2015, 3:40 p.m., Alex Merry wrote:
> > Ooh, also, please write a unit test. I can help with that if you find the 
> > idea of writing a CMake-based unit test daunting, but you can look in the 
> > tests directory for inspiration.
> 
> Marco Martin wrote:
> I'm not sure what the test should do...
> perhaps having in the repo a tarball and a source template, make it 
> generate and then compare the files?
> 
> Alex Merry wrote:
> Yes, that would work. Or you could have a source template, and check the 
> installed file can be extracted and check the extracted contents. Don't 
> forget to make sure that files you think should be excluded are in fact 
> omitted.
> 
> Marco Martin wrote:
> if i try to use kde_add_app_templates from a run_test.cmake i get the 
> message KDETemplateGenerator.cmake:97 (add_custom_target):
>   Command add_custom_target() is not scriptable
>   any idea what it could be?

Ah, you'll need to do a project-style test. See ECMPoQmToolsTest, or 
ECMInstallIconsTest. The idea is you create a CMakeLists.txt file as though it 
were a project using this module, and make, say, a check_tree.cmake script to 
check that the right things were installed in the right places. Lines 124-131 
of tests/CMakeLists.txt are what run ECMInstallIconsTest.


- Alex


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


On Dec. 16, 2015, 12:25 p.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126185/
> ---
> 
> (Updated Dec. 16, 2015, 12:25 p.m.)
> 
> 
> Review request for Build System, KDE Frameworks, Plasma, Aleix Pol Gonzalez, 
> and Simon Wächter.
> 
> 
> Repository: extra-cmake-modules
> 
> 
> Description
> ---
> 
> templates are very useful as teaching tool in order to make
> a minimal application that uses a certain framework.
> templates in the KAppTemplate repository will always get forgotten
> (plus kapptemplate is not really necessary as they work in kdevelop as well)
> An ideal situation would be frameworks having templates in their own repos
> with templates of barebone apps using the main framework features.
> In order to do that, the cmake stuff needed in order to correctly install
> a template needs to be ported to a place avaiable to all frameworks
> 
> 
> Diffs
> -
> 
>   kde-modules/KDEInstallDirs.cmake b7cd34d 
>   kde-modules/KDEPackageAppTemplates.cmake PRE-CREATION 
>   tests/CMakeLists.txt 1a66f56 
>   tests/KDEPackageAppTemplatesTest/CMakeLists.txt PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/CMakeLists.txt PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/Messages.sh PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/README PRE-CREATION 
>   
> tests/KDEPackageAppTemplatesTest/qml-plasmoid/package/contents/images/pairs.svgz
>  PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/package/contents/ui/main.qml 
> PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/package/metadata.desktop 
> PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/qml-plasmoid.kdevtemplate 
> PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/qml-plasmoid/qml-plasmoid.png PRE-CREATION 
>   tests/KDEPackageAppTemplatesTest/run_test.cmake.config PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126185/diff/
> 
> 
> Testing
> ---
> 
> done some templates installed by plasma-framework
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 126185: Make the KAppTemplate CMake module global

[Alex Merry]

> On Dec. 12, 2015, 3:35 p.m., Alex Merry wrote:
> > kde-modules/KDETemplateGenerator.cmake, line 84
> > <https://git.reviewboard.kde.org/r/126185/diff/4/?file=421458#file421458line84>
> >
> > Use ``KDE_INSTALL_KTEMPLATESDIR`` instead of 
> > ``KTEMPLATES_INSTALL_DIR``. You also need to document that this variable 
> > needs to be set before using the command.
> > 
> > My preference would actually be to make it explicit in the signature 
> > that installation will happen, by having the usage be something like
> > 
> > kde_package_app_templates(
> > TEMPLATES
> > foo
> > bar
> > baz
> > INSTALL_DIR
> > "${KDE_INSTALL_KTEMPLATESDIR}"
> > )
> > 
> > My view is that this makes it explicit what will happen when you're 
> > reading the code - it will package the templates and install them. But I'm 
> > not going to force your hand on this if you'd rather not do it that way.
> 
> Marco Martin wrote:
> would it still take ${KDE_INSTALL_KTEMPLATESDIR} as default or would be 
> mandatory to be passed by the caller?

My preference would be to make it mandatory.


> On Dec. 12, 2015, 3:35 p.m., Alex Merry wrote:
> > kde-modules/KDETemplateGenerator.cmake, line 52
> > <https://git.reviewboard.kde.org/r/126185/diff/4/?file=421458#file421458line52>
> >
> > Honestly, I'd just use ARG as the prefix - you're in a function, it's 
> > not going to leak anyway. That means you can refer to ARG_TEMPLATES, which 
> > is both shorter and more descriptive.
> > 
> > You should also check for unparsed arguments, and for the TEMPLATES 
> > argument being missing or empty. See, eg, ECMAddAppIcon.cmake for how to do 
> > this.
> 
> Alex Merry wrote:
> Oops, missed one thing here - you need to 
> ``include(CMakeParseArguments)`` earlier in the file.

You still need to do the check for unparsed arguments and for ARG_TEMPLATES not 
being set (ie: the TEMPLATES keyword being missing).


> On Dec. 12, 2015, 3:35 p.m., Alex Merry wrote:
> > kde-modules/KDETemplateGenerator.cmake, line 14
> > <https://git.reviewboard.kde.org/r/126185/diff/4/?file=421458#file421458line14>
> >
> > OK, what needs to be in the subdirectory? are template1, template2 etc 
> > the subdriectory names? The packages file name?
> > 
> > The following sentence suggests this command only creates a single 
> > template tarball, but the signature here has multiple arguments.
> > 
> > Also, the command doesn't need to include the word "template" twice. I 
> > suggest kde_add_app_template.
> > 
> > Finally, the signature now includes the TEMPLATES keyword.
> 
> Marco Martin wrote:
> the subdirectories have the uncompressed templates in it
> the subdirectory names will correspond to the final package name.

OK, so this needs to go in the documentation. I would write it as

kde_package_app_templates(TEMPLATES  [ [...]])

TEMPLATES lists subdirectories containing template files; each 
 directory will be packaged into a file named 
``.tar.bz2`` and installed to the appropriate location.

You should also either describe what should go into a template, or point to 
documentation elsewhere that describes this.


- Alex


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


On Dec. 15, 2015, 10:38 a.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126185/
> ---
> 
> (Updated Dec. 15, 2015, 10:38 a.m.)
> 
> 
> Review request for Build System, KDE Frameworks, Plasma, Aleix Pol Gonzalez, 
> and Simon Wächter.
> 
> 
> Repository: extra-cmake-modules
> 
> 
> Description
> ---
> 
> templates are very useful as teaching tool in order to make
> a minimal application that uses a certain framework.
> templates in the KAppTemplate repository will always get forgotten
> (plus kapptemplate is not really necessary as they work in kdevelop as well)
> An ideal situation would be frameworks having templates in their own repos
> with templates of barebone apps using the main framework features.
> In order to do that, the cmake stuff needed in order to correctly install
> a template needs to be port

Re: Review Request 126185: Make the KAppTemplate CMake module global

[Alex Merry]

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



kde-modules/KDETemplateGenerator.cmake (line 41)
<https://git.reviewboard.kde.org/r/126185/#comment61287>

I'd prefer ``kde_package_app_templates`` (and the module to be called 
``KDEPackageAppTemplates.cmake``).


- Alex Merry


On Dec. 15, 2015, 10:38 a.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126185/
> ---
> 
> (Updated Dec. 15, 2015, 10:38 a.m.)
> 
> 
> Review request for Build System, KDE Frameworks, Plasma, Aleix Pol Gonzalez, 
> and Simon Wächter.
> 
> 
> Repository: extra-cmake-modules
> 
> 
> Description
> ---
> 
> templates are very useful as teaching tool in order to make
> a minimal application that uses a certain framework.
> templates in the KAppTemplate repository will always get forgotten
> (plus kapptemplate is not really necessary as they work in kdevelop as well)
> An ideal situation would be frameworks having templates in their own repos
> with templates of barebone apps using the main framework features.
> In order to do that, the cmake stuff needed in order to correctly install
> a template needs to be ported to a place avaiable to all frameworks
> 
> 
> Diffs
> -
> 
>   kde-modules/KDEInstallDirs.cmake b7cd34d 
>   kde-modules/KDETemplateGenerator.cmake PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126185/diff/
> 
> 
> Testing
> ---
> 
> done some templates installed by plasma-framework
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 126185: Make the KAppTemplate CMake module global

[Alex Merry]

> On Dec. 12, 2015, 3:40 p.m., Alex Merry wrote:
> > Ooh, also, please write a unit test. I can help with that if you find the 
> > idea of writing a CMake-based unit test daunting, but you can look in the 
> > tests directory for inspiration.
> 
> Marco Martin wrote:
> I'm not sure what the test should do...
> perhaps having in the repo a tarball and a source template, make it 
> generate and then compare the files?

Yes, that would work. Or you could have a source template, and check the 
installed file can be extracted and check the extracted contents. Don't forget 
to make sure that files you think should be excluded are in fact omitted.


- Alex


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


On Dec. 15, 2015, 10:38 a.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126185/
> ---
> 
> (Updated Dec. 15, 2015, 10:38 a.m.)
> 
> 
> Review request for Build System, KDE Frameworks, Plasma, Aleix Pol Gonzalez, 
> and Simon Wächter.
> 
> 
> Repository: extra-cmake-modules
> 
> 
> Description
> ---
> 
> templates are very useful as teaching tool in order to make
> a minimal application that uses a certain framework.
> templates in the KAppTemplate repository will always get forgotten
> (plus kapptemplate is not really necessary as they work in kdevelop as well)
> An ideal situation would be frameworks having templates in their own repos
> with templates of barebone apps using the main framework features.
> In order to do that, the cmake stuff needed in order to correctly install
> a template needs to be ported to a place avaiable to all frameworks
> 
> 
> Diffs
> -
> 
>   kde-modules/KDEInstallDirs.cmake b7cd34d 
>   kde-modules/KDETemplateGenerator.cmake PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126185/diff/
> 
> 
> Testing
> ---
> 
> done some templates installed by plasma-framework
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 126185: Make the KAppTemplate CMake module global

[Alex Merry]

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


Ooh, also, please write a unit test. I can help with that if you find the idea 
of writing a CMake-based unit test daunting, but you can look in the tests 
directory for inspiration.

- Alex Merry


On Dec. 9, 2015, 12:12 p.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126185/
> ---
> 
> (Updated Dec. 9, 2015, 12:12 p.m.)
> 
> 
> Review request for Build System, KDE Frameworks, Plasma, Aleix Pol Gonzalez, 
> and Simon Wächter.
> 
> 
> Repository: extra-cmake-modules
> 
> 
> Description
> ---
> 
> templates are very useful as teaching tool in order to make
> a minimal application that uses a certain framework.
> templates in the KAppTemplate repository will always get forgotten
> (plus kapptemplate is not really necessary as they work in kdevelop as well)
> An ideal situation would be frameworks having templates in their own repos
> with templates of barebone apps using the main framework features.
> In order to do that, the cmake stuff needed in order to correctly install
> a template needs to be ported to a place avaiable to all frameworks
> 
> 
> Diffs
> -
> 
>   kde-modules/KDETemplateGenerator.cmake PRE-CREATION 
>   kde-modules/KDEInstallDirs.cmake b7cd34d 
> 
> Diff: https://git.reviewboard.kde.org/r/126185/diff/
> 
> 
> Testing
> ---
> 
> done some templates installed by plasma-framework
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 126185: Make the KAppTemplate CMake module global

[Alex Merry]

> On Dec. 12, 2015, 3:35 p.m., Alex Merry wrote:
> > kde-modules/KDETemplateGenerator.cmake, line 52
> > <https://git.reviewboard.kde.org/r/126185/diff/4/?file=421458#file421458line52>
> >
> > Honestly, I'd just use ARG as the prefix - you're in a function, it's 
> > not going to leak anyway. That means you can refer to ARG_TEMPLATES, which 
> > is both shorter and more descriptive.
> > 
> > You should also check for unparsed arguments, and for the TEMPLATES 
> > argument being missing or empty. See, eg, ECMAddAppIcon.cmake for how to do 
> > this.

Oops, missed one thing here - you need to ``include(CMakeParseArguments)`` 
earlier in the file.


- Alex


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


On Dec. 9, 2015, 12:12 p.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126185/
> ---
> 
> (Updated Dec. 9, 2015, 12:12 p.m.)
> 
> 
> Review request for Build System, KDE Frameworks, Plasma, Aleix Pol Gonzalez, 
> and Simon Wächter.
> 
> 
> Repository: extra-cmake-modules
> 
> 
> Description
> ---
> 
> templates are very useful as teaching tool in order to make
> a minimal application that uses a certain framework.
> templates in the KAppTemplate repository will always get forgotten
> (plus kapptemplate is not really necessary as they work in kdevelop as well)
> An ideal situation would be frameworks having templates in their own repos
> with templates of barebone apps using the main framework features.
> In order to do that, the cmake stuff needed in order to correctly install
> a template needs to be ported to a place avaiable to all frameworks
> 
> 
> Diffs
> -
> 
>   kde-modules/KDETemplateGenerator.cmake PRE-CREATION 
>   kde-modules/KDEInstallDirs.cmake b7cd34d 
> 
> Diff: https://git.reviewboard.kde.org/r/126185/diff/
> 
> 
> Testing
> ---
> 
> done some templates installed by plasma-framework
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 126185: Make the KAppTemplate CMake module global

[Alex Merry]

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


We're getting there :-).


kde-modules/KDEInstallDirs.cmake (line 516)
<https://git.reviewboard.kde.org/r/126185/#comment61140>

This variable should be documented.



kde-modules/KDEInstallDirs.cmake (line 518)
<https://git.reviewboard.kde.org/r/126185/#comment61139>

The (optional) 5th argument to this macro is a compatibility variable name. 
Since KTEMPLATES_INSTALL_DIR didn't exist before, it shouldn't exist now, so 
this argument should be removed.



kde-modules/KDETemplateGenerator.cmake (line 5)
<https://git.reviewboard.kde.org/r/126185/#comment61144>

I think it probably is more accurate to say that it "packages" the 
templates, rather than "generates" them. The module name should be changed to 
match as well.

You should also note that the templates are not just packaged but installed.



kde-modules/KDETemplateGenerator.cmake (lines 7 - 10)
<https://git.reviewboard.kde.org/r/126185/#comment61145>

Heh, I think I see where you copied the documentation formatting from. This 
paragraph should go.



kde-modules/KDETemplateGenerator.cmake (line 12)
<https://git.reviewboard.kde.org/r/126185/#comment61146>

"function" (singular - there is only one).



kde-modules/KDETemplateGenerator.cmake (line 14)
<https://git.reviewboard.kde.org/r/126185/#comment61142>

OK, what needs to be in the subdirectory? are template1, template2 etc the 
subdriectory names? The packages file name?

The following sentence suggests this command only creates a single template 
tarball, but the signature here has multiple arguments.

Also, the command doesn't need to include the word "template" twice. I 
suggest kde_add_app_template.

Finally, the signature now includes the TEMPLATES keyword.



kde-modules/KDETemplateGenerator.cmake (lines 16 - 17)
<https://git.reviewboard.kde.org/r/126185/#comment61141>

These are separate sentences, and should have the appropriate punctuation 
(capital letters and full stops).



kde-modules/KDETemplateGenerator.cmake (line 20)
<https://git.reviewboard.kde.org/r/126185/#comment61143>

We're way past 5.10 by now...



kde-modules/KDETemplateGenerator.cmake (lines 38 - 48)
<https://git.reviewboard.kde.org/r/126185/#comment61147>

This looks like you copied it out of KDECompilerSettings as well. It 
doesn't seem to be relevant here.



kde-modules/KDETemplateGenerator.cmake (line 52)
<https://git.reviewboard.kde.org/r/126185/#comment61148>

Honestly, I'd just use ARG as the prefix - you're in a function, it's not 
going to leak anyway. That means you can refer to ARG_TEMPLATES, which is both 
shorter and more descriptive.

You should also check for unparsed arguments, and for the TEMPLATES 
argument being missing or empty. See, eg, ECMAddAppIcon.cmake for how to do 
this.



kde-modules/KDETemplateGenerator.cmake (line 84)
<https://git.reviewboard.kde.org/r/126185/#comment61149>

Use ``KDE_INSTALL_KTEMPLATESDIR`` instead of ``KTEMPLATES_INSTALL_DIR``. 
You also need to document that this variable needs to be set before using the 
command.

My preference would actually be to make it explicit in the signature that 
installation will happen, by having the usage be something like

kde_package_app_templates(
TEMPLATES
foo
bar
baz
INSTALL_DIR
"${KDE_INSTALL_KTEMPLATESDIR}"
)

My view is that this makes it explicit what will happen when you're reading 
the code - it will package the templates and install them. But I'm not going to 
force your hand on this if you'd rather not do it that way.


- Alex Merry


On Dec. 9, 2015, 12:12 p.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126185/
> ---
> 
> (Updated Dec. 9, 2015, 12:12 p.m.)
> 
> 
> Review request for Build System, KDE Frameworks, Plasma, Aleix Pol Gonzalez, 
> and Simon Wächter.
> 
> 
> Repository: extra-cmake-modules
> 
> 
> Description
> ---
> 
> templates are very useful as teaching tool in order to make
> a minimal application that uses a certain framework.
> templates in the KAppTemplate repository will always get forgotten
> (plus kapptemplate is not really necessary as they work in kdevelop as well)
> An ideal situation would be frameworks having templates in their own repos
> with templates of barebone apps usi

Re: Review Request 126185: Make the KAppTemplate CMake module global

[Alex Merry]

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



kde-modules/KDETemplateMacro.cmake (line 1)
<https://git.reviewboard.kde.org/r/126185/#comment61041>

Use function instead of macro: this prevents variables leaking 
accidentally. That will also allow removing the leading underscores on the 
variable names.



kde-modules/KDETemplateMacro.cmake (line 2)
<https://git.reviewboard.kde.org/r/126185/#comment61043>

You might want to consider using keyword arguments (ie: 
cmake_parse_arguments). This makes the function more extensible in the future.



kde-modules/KDETemplateMacro.cmake (lines 32 - 42)
<https://git.reviewboard.kde.org/r/126185/#comment61044>

Have a look at CMake's tar mode - run `cmake -E` and look at the tar 
command. This will mean the command won't depend on external utilities like 
7-zip.



kde-modules/KDETemplateMacro.cmake (line 45)
<https://git.reviewboard.kde.org/r/126185/#comment61045>

To make this slightly more general, consider passing in the install 
destination as an argument, or at least allowing that as a possibility. The 
actual directory could (should?) be put into KDEInstallDirs.cmake as an extra 
variable.


- Alex Merry


On Dec. 3, 2015, 12:34 p.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126185/
> ---
> 
> (Updated Dec. 3, 2015, 12:34 p.m.)
> 
> 
> Review request for Build System, KDE Frameworks, Plasma, Aleix Pol Gonzalez, 
> and Simon Wächter.
> 
> 
> Repository: extra-cmake-modules
> 
> 
> Description
> ---
> 
> templates are very useful as teaching tool in order to make
> a minimal application that uses a certain framework.
> templates in the KAppTemplate repository will always get forgotten
> (plus kapptemplate is not really necessary as they work in kdevelop as well)
> An ideal situation would be frameworks having templates in their own repos
> with templates of barebone apps using the main framework features.
> In order to do that, the cmake stuff needed in order to correctly install
> a template needs to be ported to a place avaiable to all frameworks
> 
> 
> Diffs
> -
> 
>   kde-modules/KDETemplateMacro.cmake PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126185/diff/
> 
> 
> Testing
> ---
> 
> done some templates installed by plasma-framework
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 126185: Make the KAppTemplate CMake module global

[Alex Merry]

> On Dec. 3, 2015, 11:49 p.m., Aleix Pol Gonzalez wrote:
> > kde-modules/KDETemplateMacro.cmake, line 1
> > 
> >
> > You might want to add documentation.
> > 
> > You can get inspired with 
> > `kde-modules/KDEFrameworkCompilerSettings.cmake` or 
> > `modules/ECMInstallIcons.cmake`. Note it's also lacking the copyright 
> > header.

And you'll need to add a reference to the module to get Sphinx to find it - 
look at the files in docs/kde-module


- Alex


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


On Dec. 3, 2015, 12:34 p.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126185/
> ---
> 
> (Updated Dec. 3, 2015, 12:34 p.m.)
> 
> 
> Review request for Build System, KDE Frameworks, Plasma, Aleix Pol Gonzalez, 
> and Simon Wächter.
> 
> 
> Repository: extra-cmake-modules
> 
> 
> Description
> ---
> 
> templates are very useful as teaching tool in order to make
> a minimal application that uses a certain framework.
> templates in the KAppTemplate repository will always get forgotten
> (plus kapptemplate is not really necessary as they work in kdevelop as well)
> An ideal situation would be frameworks having templates in their own repos
> with templates of barebone apps using the main framework features.
> In order to do that, the cmake stuff needed in order to correctly install
> a template needs to be ported to a place avaiable to all frameworks
> 
> 
> Diffs
> -
> 
>   kde-modules/KDETemplateMacro.cmake PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126185/diff/
> 
> 
> Testing
> ---
> 
> done some templates installed by plasma-framework
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 126131: Don't delete gl texture twice in thumbnail

[Alex Merry]

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

Ship it!


Ship It!

- Alex Merry


On Nov. 22, 2015, 1:57 a.m., David Edmundson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126131/
> ---
> 
> (Updated Nov. 22, 2015, 1:57 a.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> The QSGTextures are created with
> 
> window()->createTextureFromId(m_texture, QSize(w,h),
> QuickWindow::TextureOwnsGLTexture));
> 
> this means we don't want to be deleting textures ourselves too, it will
> be deleted when we delete the QSGTexture, which is a scoped pointer
> inside our QSGNode.
> 
> BUG: 355644
> REVIEW:
> 
> 
> Diffs
> -
> 
>   src/declarativeimports/core/windowthumbnail.cpp 
> 2b09657e8ce6bd1cca1acc323d5955b2f1a1efb2 
> 
> Diff: https://git.reviewboard.kde.org/r/126131/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 125811: detect gtk engine

[Alex Merry]

> On Nov. 3, 2015, 5:38 p.m., Hrvoje Senjan wrote:
> > cmake/FindGTKEngine.cmake, line 17
> > 
> >
> > KDE_INSTALL_FULL_LIBDIR rather?

Yes, that one, please. Otherwise bad things will happen if anyone specifies an 
absolute libdir on the command line.


- Alex


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


On Oct. 27, 2015, 9:22 a.m., Jonathan Riddell wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125811/
> ---
> 
> (Updated Oct. 27, 2015, 9:22 a.m.)
> 
> 
> Review request for Plasma and David Edmundson.
> 
> 
> Repository: breeze-gtk
> 
> 
> Description
> ---
> 
> first attempt at detecting gtk 2 pixmap engine as runtime dependency
> 
> but it doesn't work if you set cmake path to different from gtk engine
> 
> and I don't know if it works on distros that use /usr/lib64/ rather than 
> /usr/lib/x86_86-linux-gnu
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 9d7b7a7 
>   cmake/FindGTKEngine.cmake PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/125811/diff/
> 
> 
> Testing
> ---
> 
> works for me unless I install outwith /usr/
> 
> 
> Thanks,
> 
> Jonathan Riddell
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 125843: Use KDE_INSTALL_DBUSINTERFACEDIR to install dbus interfaces

[Alex Merry]

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

Ship it!


Ship It!

- Alex Merry


On Oct. 28, 2015, 5:31 p.m., Heiko Becker wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125843/
> ---
> 
> (Updated Oct. 28, 2015, 5:31 p.m.)
> 
> 
> Review request for Plasma and Martin Gräßlin.
> 
> 
> Repository: kwin
> 
> 
> Description
> ---
> 
> ...and use PATH_VARS to make the config file work with absolute paths.
> 
> Two reasons to do this:
> - DBUS_INTERFACES_INSTALL_DIR is marked deprecated
> - Not hard-coding the packackage prefix is helpful on a multiarch
>   layout where the prefix is /usr/${host} but arch-independent files
>   should still be installed to /usr/share (i.e a level below the
>   prefix).
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 44a423cb723121537a2c2adaa17c1d9f8875f9b8 
>   KWinDBusInterfaceConfig.cmake.in 762a2ed20221860c50f70a1e551eb74b3b1e0f3a 
> 
> Diff: https://git.reviewboard.kde.org/r/125843/diff/
> 
> 
> Testing
> ---
> 
> The affected files are still installed in the desired place and the paths in 
> KWinDBusInterfaceConfig.cmake are correct.
> 
> 
> Thanks,
> 
> Heiko Becker
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 125844: Use PATH_VARS feature of ecm_configure_package_config_file

[Alex Merry]

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

Ship it!


Ship It!

- Alex Merry


On Oct. 28, 2015, 5:40 p.m., Heiko Becker wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125844/
> ---
> 
> (Updated Oct. 28, 2015, 5:40 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> This makes specifying absolute paths on the commandline that are then
> encoded into the config file work.
> Not hard-coding the packackage prefix is also helpful on a multiarch
> layout where the prefix is /usr/${host} but arch-independent files
> should still be installed to /usr/share (i.e a level below the
> prefix).
> 
> 
> Diffs
> -
> 
>   krunner/CMakeLists.txt e1329282fe7078b5d57f1feb2c7de960c640db7b 
>   krunner/KRunnerAppDBusInterfaceConfig.cmake.in 
> ecbfc5afc73def0303442413ae60232191fee0ff 
>   ksmserver/CMakeLists.txt a0c8852673ac9e69e2cc053107eec60122ec069b 
>   ksmserver/KSMServerDBusInterfaceConfig.cmake.in 
> e16e571bfd77ed71c53972705659a6d3053739f8 
>   ksmserver/screenlocker/CMakeLists.txt 
> 203508c63e7851d566dfbfbdc77ac29f35e588c2 
>   ksmserver/screenlocker/ScreenSaverDBusInterfaceConfig.cmake.in 
> 832677e63b2423a28ea44b3a38b8e122ee93e643 
> 
> Diff: https://git.reviewboard.kde.org/r/125844/diff/
> 
> 
> Testing
> ---
> 
> Installed, the paths in the cmake config files point correctly to the dbus 
> interface files.
> 
> 
> Thanks,
> 
> Heiko Becker
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 125845: Use KDE_INSTALL_DBUSINTERFACEDIR to install dbus interfaces

[Alex Merry]

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

Ship it!


Ship It!

- Alex Merry


On Oct. 28, 2015, 5:44 p.m., Heiko Becker wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125845/
> ---
> 
> (Updated Oct. 28, 2015, 5:44 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: khotkeys
> 
> 
> Description
> ---
> 
> ...and use PATH_VARS to make the config file work with absolute paths.
> 
> Two reasons to do this:
> - DBUS_INTERFACES_INSTALL_DIR is marked deprecated
> - Not hard-coding the packackage prefix is helpful on a multiarch
>   layout where the prefix is /usr/${host} but arch-independent files
>   should still be installed to /usr/share (i.e a level below the
>   prefix).
> 
> 
> Diffs
> -
> 
>   app/CMakeLists.txt 2e176e3c872bd066b731b907c53b6b3653901d38 
>   app/KHotKeysDBusInterfaceConfig.cmake.in 
> b217a433a242613d7443b1b2c708095df1c6d7c3 
> 
> Diff: https://git.reviewboard.kde.org/r/125845/diff/
> 
> 
> Testing
> ---
> 
> The affected dbus interface file gets installed in the desired location and 
> the path in KHotKeysDBusInterfaceConfig.cmake correctly points to it.
> 
> 
> Thanks,
> 
> Heiko Becker
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

[kde-build-metadata] /: Add kaccounts-mobile to dependency data.

[Alex Merry]

Git commit 0db5cce4e7851202e757d12f20c9125ce2c84ca8 by Alex Merry.
Committed on 18/10/2015 at 14:46.
Pushed by alexmerry into branch 'master'.

Add kaccounts-mobile to dependency data.

This is dragged in to kdesrc-build because it is in the "kde/workspace"
module, so kdesrc-build needs to know how to order it (and what branch
to build).

CCMAIL: plasma-devel@kde.org

M  +5-0dependency-data-kf5-minimum
M  +7-0dependency-data-kf5-qt5
M  +6-0logical-module-structure

http://commits.kde.org/kde-build-metadata/0db5cce4e7851202e757d12f20c9125ce2c84ca8

diff --git a/dependency-data-kf5-minimum b/dependency-data-kf5-minimum
index dab43a2..2a90a9a 100644
--- a/dependency-data-kf5-minimum
+++ b/dependency-data-kf5-minimum
@@ -1099,6 +1099,11 @@ kde/kdenetwork/kaccounts-integration: frameworks/kcmutils
 kde/kdenetwork/kaccounts-integration: frameworks/ki18n
 kde/kdenetwork/kaccounts-integration: frameworks/kcoreaddons
 kde/kdenetwork/kaccounts-integration: frameworks/kdbusaddons
+kde/kdenetwork/kaccounts-mobile: frameworks/kconfig
+kde/kdenetwork/kaccounts-mobile: frameworks/kcoreaddons
+kde/kdenetwork/kaccounts-mobile: frameworks/ki18n
+kde/kdenetwork/kaccounts-mobile: frameworks/kio
+kde/kdenetwork/kaccounts-mobile: kde/kdenetwork/kaccounts-integration
 kde/kdenetwork/kaccounts-providers: kde/kdenetwork/kaccounts-integration
 kde/kdenetwork/kaccounts-providers: frameworks/kio
 kde/kdenetwork/kaccounts-providers: frameworks/ki18n
diff --git a/dependency-data-kf5-qt5 b/dependency-data-kf5-qt5
index 0d14118..42a9be6 100644
--- a/dependency-data-kf5-qt5
+++ b/dependency-data-kf5-qt5
@@ -1192,6 +1192,13 @@ kde/kdenetwork/kaccounts-integration: frameworks/ki18n
 kde/kdenetwork/kaccounts-integration: frameworks/kcoreaddons
 kde/kdenetwork/kaccounts-integration: frameworks/kdbusaddons
 kde/kdenetwork/kaccounts-integration: frameworks/kdeclarative
+kde/kdenetwork/kaccounts-mobile: frameworks/kconfig
+kde/kdenetwork/kaccounts-mobile: frameworks/kcoreaddons
+kde/kdenetwork/kaccounts-mobile: frameworks/ki18n
+kde/kdenetwork/kaccounts-mobile: frameworks/kio
+kde/kdenetwork/kaccounts-mobile: kde/kdenetwork/kaccounts-integration
+kde/kdenetwork/kaccounts-mobile: kde/pim/kcontacts
+kde/kdenetwork/kaccounts-mobile: extragear/libs/libkgapi
 kde/kdenetwork/kaccounts-providers: kde/kdenetwork/kaccounts-integration
 kde/kdenetwork/kaccounts-providers: frameworks/kio
 kde/kdenetwork/kaccounts-providers: frameworks/ki18n
diff --git a/logical-module-structure b/logical-module-structure
index 4571add..9aa3784 100644
--- a/logical-module-structure
+++ b/logical-module-structure
@@ -1467,6 +1467,12 @@
 "kf5-qt5": "master",
 "stable-kf5-qt5": "Applications/15.08"
 },
+"kde/kdenetwork/kaccounts-mobile": {
+"stable-qt4": "",
+"latest-qt4": "",
+"kf5-qt5": "master",
+"stable-kf5-qt5": ""
+},
 "extragear/utils/krecipes": {
 "stable-qt4": "2.0",
 "latest-qt4": "master",
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Plasma Interface Guidelines

[Alex Merry]

On 2015-08-23 10:35, Jens Reuterberg wrote:

Perhaps link to the toolkit in the HIG?


The PIG was supposed to be a more Plasma-specific thing, including 
details such as listing the available names for categories of Plasmoids. 
If the content in that page is no longer useful, I'm inclined to delete 
the page, although maybe forwarding anyone who lands there to the HIG 
could be useful.


Alex



Den 23 aug 2015 10:34 fm skrev Alex Merry alex.me...@kde.org:


Techbase has a page titled Plasma Interface Guidelines[0]. Leaving
aside the unfortunate acronym, this page is sparsely populated and 
last

edited in 2012. It is also linked from the development guidelines
page[1].

Could someone have a look and decide what to do with that page, 
please?

If it's no longer useful, it should at least be removed from the
guidelines page.

Thanks

Alex

[0]: https://techbase.kde.org/Projects/Plasma/PIG
[1]: https://techbase.kde.org/Development/Guidelines
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Plasma Interface Guidelines

[Alex Merry]

Techbase has a page titled Plasma Interface Guidelines[0]. Leaving 
aside the unfortunate acronym, this page is sparsely populated and last 
edited in 2012. It is also linked from the development guidelines 
page[1].


Could someone have a look and decide what to do with that page, please? 
If it's no longer useful, it should at least be removed from the 
guidelines page.


Thanks

Alex


[0]: https://techbase.kde.org/Projects/Plasma/PIG
[1]: https://techbase.kde.org/Development/Guidelines
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 124892: bug 342962: kdeclarative plugins should be built as a bundle plugin and not a shared library

[Alex Merry]

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


+1 from me also, but could you add Harald Sitter to the reviewers (or someone 
else familiar with Linux library packaging) to confirm that it won't cause 
compatilibity issues?

- Alex Merry


On Aug. 23, 2015, 1:02 p.m., Hanspeter Niederstrasser wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/124892/
 ---
 
 (Updated Aug. 23, 2015, 1:02 p.m.)
 
 
 Review request for Build System, KDE Software on Mac OS X, KDE Frameworks, 
 and Plasma.
 
 
 Repository: kdeclarative
 
 
 Description
 ---
 
 The kdeclarative plugins (draganddropplugin, kcoreaddonsplugin, kio, 
 kquickcontrolsprivateplugin, and kquickcontrolsaddonsplugin) are being built 
 as shared libraries. They should be built as bundles (MODULE) in the 
 CMakeLists.txt file.
 
 When built as SHARED as in the current code, libdraganddropplugin.dylib gets 
 installed to $PREFIX/share/qt5/qml/org/kde/draganddrop, but is given an OS X 
 install_name of $PREFIX/lib/libdraganddropplugin.dylib. This mismatch can 
 cause problems. It is also given a compatibility_version of 0.0.0.
 
 
 Diffs
 -
 
   src/qmlcontrols/draganddrop/CMakeLists.txt e8127e4 
   src/qmlcontrols/kcoreaddons/CMakeLists.txt 3f77f2d 
   src/qmlcontrols/kioplugin/CMakeLists.txt 7b258e0 
   src/qmlcontrols/kquickcontrols/private/CMakeLists.txt da355c1 
   src/qmlcontrols/kquickcontrolsaddons/CMakeLists.txt 5b711e1 
 
 Diff: https://git.reviewboard.kde.org/r/124892/diff/
 
 
 Testing
 ---
 
 Since the plugin is not supposed to be a linkable library, it should be built 
 as MODULE in CMakeLists.txt. The physical install location remains the same 
 and plugins don't have install_names. This corrects the install_name/install 
 location mismatch. The change should not have any effect on non-OS X systems.
 
 
 Thanks,
 
 Hanspeter Niederstrasser
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Review Request 121033: Mark LibKWorkspace and ScreenSaverDBusInterface as REQUIRED

[Alex Merry]

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

Review request for Plasma, Àlex Fiestas and Aleix Pol Gonzalez.


Repository: powerdevil


Description
---

Other parts of the build system assume these have been found, so
configuring fails when they are missing.


Diffs
-

  CMakeLists.txt ebf0bff6f3aaefe5b969b033502749bddf781995 

Diff: https://git.reviewboard.kde.org/r/121033/diff/


Testing
---


Thanks,

Alex Merry

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 121033: Mark LibKWorkspace and ScreenSaverDBusInterface as REQUIRED

[Alex Merry]

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

(Updated Nov. 7, 2014, 2:49 p.m.)


Review request for Plasma, Àlex Fiestas and Aleix Pol Gonzalez.


Changes
---

Did some testing.


Repository: powerdevil


Description
---

Other parts of the build system assume these have been found, so
configuring fails when they are missing.


Diffs
-

  CMakeLists.txt ebf0bff6f3aaefe5b969b033502749bddf781995 

Diff: https://git.reviewboard.kde.org/r/121033/diff/


Testing (updated)
---

Configure errors out with a more relevant message (about not finding 
LibKWorkspace) when plasma-workspace is not installed. When plasma-workspace is 
installed, LibKWorkspace and ScreenSaverDBusInterface are correctly listed 
under the REQUIRED modules, and configuring (and building) is successful.


Thanks,

Alex Merry

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 121033: Mark LibKWorkspace and ScreenSaverDBusInterface as REQUIRED

[Alex Merry]

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

(Updated Nov. 7, 2014, 4:28 p.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma, Àlex Fiestas and Aleix Pol Gonzalez.


Repository: powerdevil


Description
---

Other parts of the build system assume these have been found, so
configuring fails when they are missing.


Diffs
-

  CMakeLists.txt ebf0bff6f3aaefe5b969b033502749bddf781995 

Diff: https://git.reviewboard.kde.org/r/121033/diff/


Testing
---

Configure errors out with a more relevant message (about not finding 
LibKWorkspace) when plasma-workspace is not installed. When plasma-workspace is 
installed, LibKWorkspace and ScreenSaverDBusInterface are correctly listed 
under the REQUIRED modules, and configuring (and building) is successful.


Thanks,

Alex Merry

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 118851: Kde-baseapps- KF5 replace generic soversion.

[Alex Merry]

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



CMakeLists.txt
https://git.reviewboard.kde.org/r/118851/#comment42289

This is not part of kf5, so shouldn't use KF5_VERSION. Instead, you may 
want a KDE_BASEAPPS_VERSION or something. This does not have to be at all 
related to the version of KF5 required.



CMakeLists.txt
https://git.reviewboard.kde.org/r/118851/#comment42291

This should probably move to the lib/konq/CMakeLists.txt file.



CMakeLists.txt
https://git.reviewboard.kde.org/r/118851/#comment42290

This should probably move the the dolphin/src/CMakeLists.txt file


- Alex Merry


On June 20, 2014, 5:53 p.m., Scarlett Clark wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118851/
 ---
 
 (Updated June 20, 2014, 5:53 p.m.)
 
 
 Review request for Dolphin, KDE Base Apps, Plasma, and Jonathan Riddell.
 
 
 Repository: kde-baseapps
 
 
 Description
 ---
 
 I was ending up with so.SOVERSION when building this, so through some 
 research I have come up with this patch to correct the issue.
 If there is a better way, please let me know.
 
 
 Diffs
 -
 
   CMakeLists.txt a5588ea 
   dolphin/src/CMakeLists.txt ce629fb 
   lib/konq/CMakeLists.txt 6857e19 
 
 Diff: https://git.reviewboard.kde.org/r/118851/diff/
 
 
 Testing
 ---
 
 Builds fine on Kubuntu Utopic frameworks chroot. Results in the expected:
 libdolphinprivate.so
 libdolphinprivate.so.4.97.0
 libdolphinprivate.so.5
 libkdeinit5_dolphin.so
 libkonq.so
 libkonq.so.4.97.0
 libkonq.so.5
 
 
 Thanks,
 
 Scarlett Clark
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 118851: Kde-baseapps- KF5 replace generic soversion.

[Alex Merry]

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



CMakeLists.txt
https://git.reviewboard.kde.org/r/118851/#comment42292

But now you'll want to put an actual version number in here (or remove the 
variable use altogether).


- Alex Merry


On June 20, 2014, 6:44 p.m., Scarlett Clark wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118851/
 ---
 
 (Updated June 20, 2014, 6:44 p.m.)
 
 
 Review request for Dolphin, KDE Base Apps, Plasma, and Jonathan Riddell.
 
 
 Repository: kde-baseapps
 
 
 Description
 ---
 
 I was ending up with so.SOVERSION when building this, so through some 
 research I have come up with this patch to correct the issue.
 If there is a better way, please let me know.
 
 
 Diffs
 -
 
   CMakeLists.txt a5588ea 
   dolphin/src/CMakeLists.txt ce629fb 
   lib/konq/CMakeLists.txt 6857e19 
 
 Diff: https://git.reviewboard.kde.org/r/118851/diff/
 
 
 Testing
 ---
 
 Builds fine on Kubuntu Utopic frameworks chroot. Results in the expected:
 libdolphinprivate.so
 libdolphinprivate.so.4.97.0
 libdolphinprivate.so.5
 libkdeinit5_dolphin.so
 libkonq.so
 libkonq.so.4.97.0
 libkonq.so.5
 
 
 Thanks,
 
 Scarlett Clark
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 118851: Kde-baseapps- KF5 replace generic soversion.

[Alex Merry]

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

Ship it!


Ship It!

- Alex Merry


On June 20, 2014, 6:44 p.m., Scarlett Clark wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118851/
 ---
 
 (Updated June 20, 2014, 6:44 p.m.)
 
 
 Review request for Dolphin, KDE Base Apps, Plasma, and Jonathan Riddell.
 
 
 Repository: kde-baseapps
 
 
 Description
 ---
 
 I was ending up with so.SOVERSION when building this, so through some 
 research I have come up with this patch to correct the issue.
 If there is a better way, please let me know.
 
 
 Diffs
 -
 
   CMakeLists.txt a5588ea 
   dolphin/src/CMakeLists.txt ce629fb 
   lib/konq/CMakeLists.txt 6857e19 
 
 Diff: https://git.reviewboard.kde.org/r/118851/diff/
 
 
 Testing
 ---
 
 Builds fine on Kubuntu Utopic frameworks chroot. Results in the expected:
 libdolphinprivate.so
 libdolphinprivate.so.4.97.0
 libdolphinprivate.so.5
 libkdeinit5_dolphin.so
 libkonq.so
 libkonq.so.4.97.0
 libkonq.so.5
 
 
 Thanks,
 
 Scarlett Clark
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 118476: Move kded module to kf5/kded subdirectory

[Alex Merry]

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

(Updated June 4, 2014, 9:16 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Plasma.


Repository: plasma-framework


Description
---

Move kded module to kf5/kded subdirectory

This makes sure the module is properly versioned.


Diffs
-

  src/platformstatus/CMakeLists.txt 676a8be30df41b042758d55b58caa22fc6c89311 
  src/platformstatus/kded_platformstatus.desktop 
9976fa3041a6c12fc4b3ce2146fb481181fa65b8 

Diff: https://git.reviewboard.kde.org/r/118476/diff/


Testing
---

qdbus org.kde.kded5 /kded loadModule kded_platformstatus
succeeds


Thanks,

Alex Merry

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Review Request 118476: Move kded module to kf5/kded subdirectory

[Alex Merry]

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

Review request for KDE Frameworks and Plasma.


Repository: plasma-framework


Description
---

Move kded module to kf5/kded subdirectory

This makes sure the module is properly versioned.


Diffs
-

  src/platformstatus/CMakeLists.txt 676a8be30df41b042758d55b58caa22fc6c89311 
  src/platformstatus/kded_platformstatus.desktop 
9976fa3041a6c12fc4b3ce2146fb481181fa65b8 

Diff: https://git.reviewboard.kde.org/r/118476/diff/


Testing
---

qdbus org.kde.kded5 /kded loadModule kded_platformstatus
succeeds


Thanks,

Alex Merry

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 118340: Allow the kactivitymanagerd daemon to be disabled.

[Alex Merry]

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


I'm happy with this. I think this should go in, even if there isn't another 
release, because packagers can pick it up as an official patch if they want. 
But I'll let Ivan have the final say.

- Alex Merry


On May 31, 2014, 3:42 a.m., Matthew Dawson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118340/
 ---
 
 (Updated May 31, 2014, 3:42 a.m.)
 
 
 Review request for KDE Frameworks, Plasma and Ivan Čukić.
 
 
 Repository: kactivities
 
 
 Description
 ---
 
 To allow for the library to be co-installed with the frameworks version, 
 allow the daemon to be disabled.
 
 I'm not sure if this is the best way to do this.  If there is any better way, 
 I'll update the patch as appropriate.
 
 
 Diffs
 -
 
   src/CMakeLists.txt b4733648fd53ebd681694f185cc5b23f51dfd3f9 
 
 Diff: https://git.reviewboard.kde.org/r/118340/diff/
 
 
 Testing
 ---
 
 Visually inspected install directories.  Seems to remove only what is 
 necessary.
 
 
 Thanks,
 
 Matthew Dawson
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 118155: adapt to ecm_add_tests so that tests can be found

[Alex Merry]

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

Ship it!


Builds and tests pass for me on Linux. I think we don't need the NAME_PREFIX 
arguments any more, but I'll wait to see what people think of 
https://git.reviewboard.kde.org/r/118385/ first.

- Alex Merry


On May 15, 2014, 3 p.m., Patrick Spendrin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118155/
 ---
 
 (Updated May 15, 2014, 3 p.m.)
 
 
 Review request for KDE Frameworks, kdewin and Plasma.
 
 
 Repository: plasma-framework
 
 
 Description
 ---
 
 adapt to ecm_add_tests so that tests can be found
 
 
 Diffs
 -
 
   autotests/CMakeLists.txt dcee37f0771753d3e381e9d77f351cff16531e93 
 
 Diff: https://git.reviewboard.kde.org/r/118155/diff/
 
 
 Testing
 ---
 
 mingw
 
 
 Thanks,
 
 Patrick Spendrin
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 118340: Allow the kactivitymanagerd daemon to be disabled.

[Alex Merry]

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


Did you try compiling this? Because that macro doesn't exist any more - there 
is an ecm_optional_add_subdirectory() in ECM if you 
include(ECMOptionalAddSubdirectory), though.

However, I think an explicit option(), with a useful help-text, would be better.

- Alex Merry


On May 27, 2014, 5:27 a.m., Matthew Dawson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118340/
 ---
 
 (Updated May 27, 2014, 5:27 a.m.)
 
 
 Review request for KDE Frameworks, Plasma and Ivan Čukić.
 
 
 Repository: kactivities
 
 
 Description
 ---
 
 To allow for the library to be co-installed with the frameworks version, 
 allow the daemon to be disabled.
 
 I'm not sure if this is the best way to do this.  If there is any better way, 
 I'll update the patch as appropriate.
 
 
 Diffs
 -
 
   src/CMakeLists.txt b4733648fd53ebd681694f185cc5b23f51dfd3f9 
 
 Diff: https://git.reviewboard.kde.org/r/118340/diff/
 
 
 Testing
 ---
 
 Visually inspected install directories.  Seems to remove only what is 
 necessary.
 
 
 Thanks,
 
 Matthew Dawson
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 118340: Allow the kactivitymanagerd daemon to be disabled.

[Alex Merry]

 On May 29, 2014, 3:02 p.m., Alex Merry wrote:
  Did you try compiling this? Because that macro doesn't exist any more - 
  there is an ecm_optional_add_subdirectory() in ECM if you 
  include(ECMOptionalAddSubdirectory), though.
  
  However, I think an explicit option(), with a useful help-text, would be 
  better.

Ah, seeing David's comment made me realise I hadn't clocked the branch (I 
generally assume anything the kdeframeworks group has been added to is a 
frameworks branch). Although my point still stands about using option() to get 
a more useful help-text.


- Alex


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


On May 27, 2014, 5:27 a.m., Matthew Dawson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118340/
 ---
 
 (Updated May 27, 2014, 5:27 a.m.)
 
 
 Review request for KDE Frameworks, Plasma and Ivan Čukić.
 
 
 Repository: kactivities
 
 
 Description
 ---
 
 To allow for the library to be co-installed with the frameworks version, 
 allow the daemon to be disabled.
 
 I'm not sure if this is the best way to do this.  If there is any better way, 
 I'll update the patch as appropriate.
 
 
 Diffs
 -
 
   src/CMakeLists.txt b4733648fd53ebd681694f185cc5b23f51dfd3f9 
 
 Diff: https://git.reviewboard.kde.org/r/118340/diff/
 
 
 Testing
 ---
 
 Visually inspected install directories.  Seems to remove only what is 
 necessary.
 
 
 Thanks,
 
 Matthew Dawson
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Rebuild everything

[Alex Merry]

Over the course of the KF5 sprint, we changed a bunch of installation
directories, including those used by KService, KNotifications and
KPluginLoader. As a result, I suggest you do a full rebuild from
extra-cmake-modules upwards. If you only rebuild some stuff, important
files may not be found and/or the configure step may fail.

Sorry this was so late in the cycle.

Alex
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 117762: Improve XCB dependency handling

[Alex Merry]

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



CMakeLists.txt
https://git.reviewboard.kde.org/r/117762/#comment39419

What you actually want is to use OPTIONAL_COMPONENTS, I reckon. This can be 
combined with the previous find_package call.

The issue is that you change the FOUND status of XCB as a whole if you 
search for a COMPONENT that is not found, even if you do it in a separate 
find_package call.


- Alex Merry


On April 25, 2014, 8:16 a.m., Martin Gräßlin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/117762/
 ---
 
 (Updated April 25, 2014, 8:16 a.m.)
 
 
 Review request for Plasma.
 
 
 Repository: plasma-desktop
 
 
 Description
 ---
 
 * Only find what is really needed
 * XCB-XKB as optional dependency for the keyboard kcm
 
 
 Diffs
 -
 
   CMakeLists.txt c548c0e 
   kcms/CMakeLists.txt 3b21c2f 
   kcms/input/CMakeLists.txt 1257b80 
   kcms/kfontinst/lib/CMakeLists.txt a133956 
 
 Diff: https://git.reviewboard.kde.org/r/117762/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Martin Gräßlin
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Review Request 117584: Standardise DBus service CMake code

[Alex Merry]

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

Review request for Plasma.


Repository: plasma-workspace


Description
---

Standardise DBus service CMake code

Use configure_file() and install() instead of various macros.


See https://git.reviewboard.kde.org/r/117581/


Diffs
-

  kuiserver/CMakeLists.txt 5406c2e3663bad98839d9ced3c202969422e64dd 
  kglobalaccel/CMakeLists.txt 9606293e1ecbcccefae11bafb4083a181c2a24ba 
  krunner/CMakeLists.txt afd5495a39e64bb5ac6f379c6da05d01533df815 
  CMakeLists.txt cc591200e719b830cc9b4b6480069851f79743eb 

Diff: https://git.reviewboard.kde.org/r/117584/diff/


Testing
---

Configured, built, installed.  Checked (in `make install` output) that files 
were installed to correct location.  Visually inspected installed files.


Thanks,

Alex Merry

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 116907: Implemented Player MPRIS spec adaptor

[Alex Merry]

 On March 26, 2014, 6:09 p.m., Alex Merry wrote:
  All looks sane.  Is this Qt4 or Qt5?  Because you've got a KUrl in the 
  added code...
 
 Bhushan Shah wrote:
 Qt4

OK, that's fine then.


- Alex


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


On March 26, 2014, 5:23 p.m., Ashish Madeti wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/116907/
 ---
 
 (Updated March 26, 2014, 5:23 p.m.)
 
 
 Review request for Plasma, Alex Merry, Eike Hein, Nikolaos Chatzidakis, 
 Shantanu Tushar, Sinny Kumari, and Sujith Haridasan.
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 Implemented Player DBus adaptor of MPRIS specifications for Plasma 
 Mediacenter.
 Specification reference: 
 http://specifications.freedesktop.org/mpris-spec/latest/Player_Interface.html
 
 Some more work needs to be done in the adaptor which I plan to do soon.
 
 
 Diffs
 -
 
   libs/mpris2/mediaplayer2.h e68bc5c 
   libs/mpris2/mediaplayer2.cpp ff96618 
   libs/mpris2/mediaplayer2player.h 203d681 
   libs/mpris2/mediaplayer2player.cpp 7871efa 
   libs/mpris2/mpris2.cpp a8ad3ef 
   mediaelements/mediaplayer/MediaPlayer.qml 39ed617 
   shells/newshell/mainwindow.cpp d2d71d4 
   shells/newshell/package/contents/ui/mediacenter.qml bac33c2 
 
 Diff: https://git.reviewboard.kde.org/r/116907/diff/
 
 
 Testing
 ---
 
 Tested with qdbusviewer, the properties and methods are working fine.
 
 
 Thanks,
 
 Ashish Madeti
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 116907: Implemented Player MPRIS spec adaptor

[Alex Merry]

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


All looks sane.  Is this Qt4 or Qt5?  Because you've got a KUrl in the added 
code...

- Alex Merry


On March 26, 2014, 5:23 p.m., Ashish Madeti wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/116907/
 ---
 
 (Updated March 26, 2014, 5:23 p.m.)
 
 
 Review request for Plasma, Alex Merry, Eike Hein, Nikolaos Chatzidakis, 
 Shantanu Tushar, Sinny Kumari, and Sujith Haridasan.
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 Implemented Player DBus adaptor of MPRIS specifications for Plasma 
 Mediacenter.
 Specification reference: 
 http://specifications.freedesktop.org/mpris-spec/latest/Player_Interface.html
 
 Some more work needs to be done in the adaptor which I plan to do soon.
 
 
 Diffs
 -
 
   libs/mpris2/mediaplayer2.h e68bc5c 
   libs/mpris2/mediaplayer2.cpp ff96618 
   libs/mpris2/mediaplayer2player.h 203d681 
   libs/mpris2/mediaplayer2player.cpp 7871efa 
   libs/mpris2/mpris2.cpp a8ad3ef 
   mediaelements/mediaplayer/MediaPlayer.qml 39ed617 
   shells/newshell/mainwindow.cpp d2d71d4 
   shells/newshell/package/contents/ui/mediacenter.qml bac33c2 
 
 Diff: https://git.reviewboard.kde.org/r/116907/diff/
 
 
 Testing
 ---
 
 Tested with qdbusviewer, the properties and methods are working fine.
 
 
 Thanks,
 
 Ashish Madeti
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Review Request 117060: Explicitly specify link interface libraries for libKF5PlasmaQuick

[Alex Merry]

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

Review request for KDE Frameworks and Plasma.


Repository: plasma-framework


Description
---

Explicitly specify link interface libraries for libKF5PlasmaQuick

The headers may not be public (yet?), but it doesn't hurt to have this
stuff specified properly.


Diffs
-

  src/plasmaquick/CMakeLists.txt 83af22ecaf235c26115aa741d3c288bc71c269e9 

Diff: https://git.reviewboard.kde.org/r/117060/diff/


Testing
---

Builds and installs.  kde-runtime and kde-workspace then also build.


Thanks,

Alex Merry

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Review Request 117059: Explicitly state link interface for libKF5Activites

[Alex Merry]

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

Review request for KDE Frameworks and Plasma.


Repository: kactivities


Description
---

Explicitly state link interface for libKF5Activites


Diffs
-

  src/lib/core/CMakeLists.txt 7edd2e34bc5ccec6ae526c0305d090afdaf9f306 

Diff: https://git.reviewboard.kde.org/r/117059/diff/


Testing
---

Compiles, installs.  plasa-framework, kde-runtime and kde-workspace then also 
build.


Thanks,

Alex Merry

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 117059: Explicitly state link interface for libKF5Activites

[Alex Merry]

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

(Updated March 25, 2014, 7:39 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Plasma.


Repository: kactivities


Description
---

Explicitly state link interface for libKF5Activites


Diffs
-

  src/lib/core/CMakeLists.txt 7edd2e34bc5ccec6ae526c0305d090afdaf9f306 

Diff: https://git.reviewboard.kde.org/r/117059/diff/


Testing
---

Compiles, installs.  plasa-framework, kde-runtime and kde-workspace then also 
build.


Thanks,

Alex Merry

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 117060: Explicitly specify link interface libraries for libKF5PlasmaQuick

[Alex Merry]

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

(Updated March 25, 2014, 7:40 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Plasma.


Repository: plasma-framework


Description
---

Explicitly specify link interface libraries for libKF5PlasmaQuick

The headers may not be public (yet?), but it doesn't hurt to have this
stuff specified properly.


Diffs
-

  src/plasmaquick/CMakeLists.txt 83af22ecaf235c26115aa741d3c288bc71c269e9 

Diff: https://git.reviewboard.kde.org/r/117060/diff/


Testing
---

Builds and installs.  kde-runtime and kde-workspace then also build.


Thanks,

Alex Merry

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 116907: Implemented Player MPRIS spec adaptor

[Alex Merry]

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


Could you test it with https://github.com/randomguy3/mpristester please?  It's 
much more comprehensive than the now playing applet.

- Alex Merry


On March 25, 2014, 7:02 p.m., Ashish Madeti wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/116907/
 ---
 
 (Updated March 25, 2014, 7:02 p.m.)
 
 
 Review request for Plasma, Shantanu Tushar, Sinny Kumari, and Sujith 
 Haridasan.
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 Implemented Player DBus adaptor of MPRIS specifications for Plasma 
 Mediacenter.
 Specification reference: 
 http://specifications.freedesktop.org/mpris-spec/latest/Player_Interface.html
 
 Some more work needs to be done in the adaptor which I plan to do soon.
 
 
 Diffs
 -
 
   shells/newshell/mainwindow.cpp d2d71d4 
   shells/newshell/package/contents/ui/mediacenter.qml bac33c2 
   libs/mpris2/mediaplayer2.h e68bc5c 
   libs/mpris2/mediaplayer2.cpp ff96618 
   libs/mpris2/mediaplayer2player.h 203d681 
   libs/mpris2/mediaplayer2player.cpp 7871efa 
   libs/mpris2/mpris2.cpp a8ad3ef 
   mediaelements/playlist/Playlist.qml 5dde297 
 
 Diff: https://git.reviewboard.kde.org/r/116907/diff/
 
 
 Testing
 ---
 
 Tested with qdbusviewer, the properties and methods are working fine.
 
 
 Thanks,
 
 Ashish Madeti
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Review Request 116931: Make sure kdeinit starts kded

[Alex Merry]

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

Review request for Plasma and Sebastian Kügler.


Repository: kde-workspace


Description
---

Make sure kdeinit starts kded

We now have to explicitly tell kdeinit to start kded using the --kded
option.


Diffs
-

  plasma-workspace/startkde/startkde.cmake 
86abf5b68dc5717d9fc257e0d442242fc18d3058 

Diff: https://git.reviewboard.kde.org/r/116931/diff/


Testing
---


Thanks,

Alex Merry

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 116931: Make sure kdeinit starts kded

[Alex Merry]

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

(Updated March 20, 2014, 4:04 p.m.)


Review request for Plasma and Sebastian Kügler.


Repository: kde-workspace


Description (updated)
---

Make sure kdeinit starts kded

We now have to explicitly tell kdeinit to start kded using the --kded
option (as of https://git.reviewboard.kde.org/r/116928/).


Diffs
-

  plasma-workspace/startkde/startkde.cmake 
86abf5b68dc5717d9fc257e0d442242fc18d3058 

Diff: https://git.reviewboard.kde.org/r/116931/diff/


Testing
---


Thanks,

Alex Merry

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 116931: Make sure kdeinit starts kded

[Alex Merry]

 On March 20, 2014, 4:29 p.m., Àlex Fiestas wrote:
  kdeinit should be launching kded, if not what has changed?

https://git.reviewboard.kde.org/r/116928/ changes it to not load it by default, 
so that applications running outside of Plasma don't run it just because they 
wanted a kioslave.  It should still be auto-started if anything tries to use 
one of its D-Bus interfaces, though.


- Alex


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


On March 20, 2014, 4:04 p.m., Alex Merry wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/116931/
 ---
 
 (Updated March 20, 2014, 4:04 p.m.)
 
 
 Review request for Plasma and Sebastian Kügler.
 
 
 Repository: kde-workspace
 
 
 Description
 ---
 
 Make sure kdeinit starts kded
 
 We now have to explicitly tell kdeinit to start kded using the --kded
 option (as of https://git.reviewboard.kde.org/r/116928/).
 
 
 Diffs
 -
 
   plasma-workspace/startkde/startkde.cmake 
 86abf5b68dc5717d9fc257e0d442242fc18d3058 
 
 Diff: https://git.reviewboard.kde.org/r/116931/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Alex Merry
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 116931: Make sure kdeinit starts kded

[Alex Merry]

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

(Updated March 20, 2014, 7:31 p.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma and Sebastian Kügler.


Repository: kde-workspace


Description
---

Make sure kdeinit starts kded

We now have to explicitly tell kdeinit to start kded using the --kded
option (as of https://git.reviewboard.kde.org/r/116928/).


Diffs
-

  plasma-workspace/startkde/startkde.cmake 
86abf5b68dc5717d9fc257e0d442242fc18d3058 

Diff: https://git.reviewboard.kde.org/r/116931/diff/


Testing
---


Thanks,

Alex Merry

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 114260: Port mouse dataengine

[Alex Merry]

 On Feb. 2, 2014, 11:42 a.m., Bhushan Shah wrote:
  Please test it with plasmaengineexplorer in my testing it shows no sources.

Any progress on this?


- Alex


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


On Feb. 2, 2014, 11:26 a.m., Andrea Scarpino wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/114260/
 ---
 
 (Updated Feb. 2, 2014, 11:26 a.m.)
 
 
 Review request for KDE Frameworks and Plasma.
 
 
 Repository: kde-workspace
 
 
 Description
 ---
 
 = subject.
 
 I commented the lines about scheduleSourcesUpdated(). What to do with them?
 
 
 Diffs
 -
 
   plasma/generic/dataengines/mouse/mouseengine.cpp 
 19a7fb7405647963ebadb4f71104e8164b729df7 
   plasma/generic/dataengines/mouse/mouseengine.h 
 d55565dc3875b794dce14054d9a8b77cef9b2347 
   plasma/generic/dataengines/mouse/cursornotificationhandler.cpp 
 3cb9add2ec1d4cb4d5904a381b8ba8f29279e05b 
   plasma/generic/dataengines/CMakeLists.txt 
 5e9f11bcef806576946257bdeaa40efb445b7daf 
   plasma/generic/dataengines/mouse/CMakeLists.txt 
 29cab878bd1fb514d7ac025134fbfb58a5a1376e 
   plasma/generic/dataengines/mouse/cursornotificationhandler.h 
 3b571f8f5ffe6db4efeef7827acf003911bcd5dc 
 
 Diff: https://git.reviewboard.kde.org/r/114260/diff/
 
 
 Testing
 ---
 
 Builds
 
 
 Thanks,
 
 Andrea Scarpino
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 114260: Port mouse dataengine

[Alex Merry]

 On Dec. 8, 2013, 12:08 p.m., Bhushan Shah wrote:
  plasma/generic/dataengines/mouse/mouseengine.cpp, line 34
  https://git.reviewboard.kde.org/r/114260/diff/2/?file=222312#file222312line34
 
  You should call init() here. Have a look at 
  http://community.kde.org/Plasma/PortingTolibplasma2#DataEngine for changes 
  in API.

Any update on this?


- Alex


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


On Dec. 3, 2013, 9:51 a.m., Andrea Scarpino wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/114260/
 ---
 
 (Updated Dec. 3, 2013, 9:51 a.m.)
 
 
 Review request for KDE Frameworks and Plasma.
 
 
 Repository: kde-workspace
 
 
 Description
 ---
 
 = subject.
 
 I commented the lines about scheduleSourcesUpdated(). What to do with them?
 
 
 Diffs
 -
 
   plasma/generic/dataengines/CMakeLists.txt 3e94325 
   plasma/generic/dataengines/mouse/CMakeLists.txt 29cab87 
   plasma/generic/dataengines/mouse/cursornotificationhandler.h 3b571f8 
   plasma/generic/dataengines/mouse/cursornotificationhandler.cpp 3cb9add 
   plasma/generic/dataengines/mouse/mouseengine.h d55565d 
   plasma/generic/dataengines/mouse/mouseengine.cpp 19a7fb7 
 
 Diff: https://git.reviewboard.kde.org/r/114260/diff/
 
 
 Testing
 ---
 
 Builds
 
 
 Thanks,
 
 Andrea Scarpino
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Notes from KRunner Breakout

[Alex Merry]

On 16/01/14 20:15, Vishesh Handa wrote:
 * Discovering and managing runners is quite hard. A new KCM should be created 
 for that purpose.

I would also add that runner help is quite hard to navigate at the
moment; some thought probably needs to go into how that should work (it
would be nice if it was searchable, for example).

It's possible Milou already has this figured out; I haven't actually
tried it out.

Alex
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 114683: Clean up target_link_libraries for KF5Plasma

[Alex Merry]

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

(Updated Dec. 27, 2013, 4:29 p.m.)


Status
--

This change has been marked as submitted.


Review request for Build System, KDE Frameworks and Plasma.


Repository: plasma-framework


Description
---

Clean up target_link_libraries for KF5Plasma

It is now a single call using PUBLIC and PRIVATE keywords.  This removes
a CMake warning about using LINK_INTERFACE_LIBRARIES.


Diffs
-

  src/plasma/CMakeLists.txt 5a6ceabb3d6b95fb792e19aab034b87d0dd688a9 

Diff: https://git.reviewboard.kde.org/r/114683/diff/


Testing
---

plasma-framework and kde-workspace both still compile.


Thanks,

Alex Merry

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Framework licenses

[Alex Merry]

On 23/12/13 18:46, Aurélien Gâteau wrote:
 - plasma-framework: this framework uses multiple licenses, but majority is 
 LGPL (this is the case for many other frameworks). Should we use a LGPL 2.1 
 COPYING.LIB file?

A quick attack with git grep shows the majority of code to be LGPL 2
(+), with the odd LGPL 2.1 (+) file, in the libraries, along with some
GPL2 code (in the examples etc).  As such, I committed an LGPL 2.1
COPYING.LIB file (since that should work for all the LGPL files) and a
GPL2 COPYING file.

Alex
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Review Request 113993: Remove the nowplaying dataengine

[Alex Merry]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/113993/
---

Review request for kde-workspace and Plasma.


Repository: kde-workspace


Description
---

Remove the nowplaying dataengine

This has been superceded by the mpris2 dataengine.


(And good riddance)


Diffs
-

  plasma/generic/dataengines/CMakeLists.txt ee010a9 
  plasma/generic/dataengines/nowplaying/CMakeLists.txt ebb9e8e 
  plasma/generic/dataengines/nowplaying/Messages.sh eaac8ef 
  plasma/generic/dataengines/nowplaying/TODO 557902f 
  plasma/generic/dataengines/nowplaying/config-nowplaying.cmake c596066 
  plasma/generic/dataengines/nowplaying/nowplaying.operations 4a5587d 
  plasma/generic/dataengines/nowplaying/nowplayingengine.h 780233c 
  plasma/generic/dataengines/nowplaying/nowplayingengine.cpp f88d739 
  plasma/generic/dataengines/nowplaying/plasma-dataengine-nowplaying.desktop 
7717082 
  plasma/generic/dataengines/nowplaying/playeractionjob.h 0cfbf1b 
  plasma/generic/dataengines/nowplaying/playeractionjob.cpp 6943c25 
  plasma/generic/dataengines/nowplaying/playercontainer.h 06360e0 
  plasma/generic/dataengines/nowplaying/playercontainer.cpp 79db747 
  plasma/generic/dataengines/nowplaying/playercontrol.h 760498d 
  plasma/generic/dataengines/nowplaying/playercontrol.cpp 7480af1 
  plasma/generic/dataengines/nowplaying/playerinterface/dbuswatcher.h 38522ef 
  plasma/generic/dataengines/nowplaying/playerinterface/dbuswatcher.cpp 3e7396a 
  plasma/generic/dataengines/nowplaying/playerinterface/juk.h 5f44618 
  plasma/generic/dataengines/nowplaying/playerinterface/juk.cpp ad3deb6 
  plasma/generic/dataengines/nowplaying/playerinterface/juk_p.h d8f672c 
  plasma/generic/dataengines/nowplaying/playerinterface/mpris/mpris.h cb8cf83 
  plasma/generic/dataengines/nowplaying/playerinterface/mpris/mpris.cpp 69c3c3c 
  plasma/generic/dataengines/nowplaying/playerinterface/mpris/mpris_p.h d8c3121 
  plasma/generic/dataengines/nowplaying/playerinterface/mpris/mprisdbustypes.h 
325f3e4 
  
plasma/generic/dataengines/nowplaying/playerinterface/mpris/mprisdbustypes.cpp 
2d42a50 
  
plasma/generic/dataengines/nowplaying/playerinterface/mpris/org.freedesktop.MediaPlayer.player.xml
 302c383 
  
plasma/generic/dataengines/nowplaying/playerinterface/mpris/org.freedesktop.MediaPlayer.root.xml
 aa93e4b 
  
plasma/generic/dataengines/nowplaying/playerinterface/mpris/org.freedesktop.MediaPlayer.tracklist.xml
 e8cb127 
  plasma/generic/dataengines/nowplaying/playerinterface/mpris2/mpris2.h 0ab3650 
  plasma/generic/dataengines/nowplaying/playerinterface/mpris2/mpris2.cpp 
fbc0689 
  plasma/generic/dataengines/nowplaying/playerinterface/mpris2/mpris2_p.h 
04a3a76 
  plasma/generic/dataengines/nowplaying/playerinterface/org.kde.juk.player.xml 
bb6e18d 
  plasma/generic/dataengines/nowplaying/playerinterface/player.h 9c2e0fc 
  plasma/generic/dataengines/nowplaying/playerinterface/player.cpp b0f2680 
  plasma/generic/dataengines/nowplaying/playerinterface/playerfactory.h 27371f3 
  plasma/generic/dataengines/nowplaying/playerinterface/playerfactory.cpp 
53c1724 
  plasma/generic/dataengines/nowplaying/playerinterface/pollingwatcher.h 
34446da 
  plasma/generic/dataengines/nowplaying/playerinterface/pollingwatcher.cpp 
cf6bd99 
  plasma/generic/dataengines/nowplaying/playerinterface/xmms.h 8e58c73 
  plasma/generic/dataengines/nowplaying/playerinterface/xmms.cpp da3e9db 
  plasma/generic/dataengines/nowplaying/playerinterface/xmms_p.h 10a6e51 

Diff: http://git.reviewboard.kde.org/r/113993/diff/


Testing
---

CMake still runs successfully.


Thanks,

Alex Merry

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 113993: Remove the nowplaying dataengine

[Alex Merry]

On 21/11/13 11:02, Alex Merry wrote:
 Review request for kde-workspace and Plasma.
 By Alex Merry.
 *Repository: * kde-workspace
 
 
   Description
 
 Remove the nowplaying dataengine
 
 This has been superceded by the mpris2 dataengine.

Oh, a question about this: working on KF5 has got me in the habit of
posting everything on reviewboard.  Is this the right approach with
Plasma?  Because I'm the maintainer of this engine (and the mpris2
engine, which I'm going to try re-enabling at some point soon).

Alex
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 113993: Remove the nowplaying dataengine

[Alex Merry]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/113993/
---

(Updated Nov. 21, 2013, 11:33 a.m.)


Status
--

This change has been marked as submitted.


Review request for kde-workspace and Plasma.


Repository: kde-workspace


Description
---

Remove the nowplaying dataengine

This has been superceded by the mpris2 dataengine.


(And good riddance)


Diffs
-

  plasma/generic/dataengines/CMakeLists.txt ee010a9 
  plasma/generic/dataengines/nowplaying/CMakeLists.txt ebb9e8e 
  plasma/generic/dataengines/nowplaying/Messages.sh eaac8ef 
  plasma/generic/dataengines/nowplaying/TODO 557902f 
  plasma/generic/dataengines/nowplaying/config-nowplaying.cmake c596066 
  plasma/generic/dataengines/nowplaying/nowplaying.operations 4a5587d 
  plasma/generic/dataengines/nowplaying/nowplayingengine.h 780233c 
  plasma/generic/dataengines/nowplaying/nowplayingengine.cpp f88d739 
  plasma/generic/dataengines/nowplaying/plasma-dataengine-nowplaying.desktop 
7717082 
  plasma/generic/dataengines/nowplaying/playeractionjob.h 0cfbf1b 
  plasma/generic/dataengines/nowplaying/playeractionjob.cpp 6943c25 
  plasma/generic/dataengines/nowplaying/playercontainer.h 06360e0 
  plasma/generic/dataengines/nowplaying/playercontainer.cpp 79db747 
  plasma/generic/dataengines/nowplaying/playercontrol.h 760498d 
  plasma/generic/dataengines/nowplaying/playercontrol.cpp 7480af1 
  plasma/generic/dataengines/nowplaying/playerinterface/dbuswatcher.h 38522ef 
  plasma/generic/dataengines/nowplaying/playerinterface/dbuswatcher.cpp 3e7396a 
  plasma/generic/dataengines/nowplaying/playerinterface/juk.h 5f44618 
  plasma/generic/dataengines/nowplaying/playerinterface/juk.cpp ad3deb6 
  plasma/generic/dataengines/nowplaying/playerinterface/juk_p.h d8f672c 
  plasma/generic/dataengines/nowplaying/playerinterface/mpris/mpris.h cb8cf83 
  plasma/generic/dataengines/nowplaying/playerinterface/mpris/mpris.cpp 69c3c3c 
  plasma/generic/dataengines/nowplaying/playerinterface/mpris/mpris_p.h d8c3121 
  plasma/generic/dataengines/nowplaying/playerinterface/mpris/mprisdbustypes.h 
325f3e4 
  
plasma/generic/dataengines/nowplaying/playerinterface/mpris/mprisdbustypes.cpp 
2d42a50 
  
plasma/generic/dataengines/nowplaying/playerinterface/mpris/org.freedesktop.MediaPlayer.player.xml
 302c383 
  
plasma/generic/dataengines/nowplaying/playerinterface/mpris/org.freedesktop.MediaPlayer.root.xml
 aa93e4b 
  
plasma/generic/dataengines/nowplaying/playerinterface/mpris/org.freedesktop.MediaPlayer.tracklist.xml
 e8cb127 
  plasma/generic/dataengines/nowplaying/playerinterface/mpris2/mpris2.h 0ab3650 
  plasma/generic/dataengines/nowplaying/playerinterface/mpris2/mpris2.cpp 
fbc0689 
  plasma/generic/dataengines/nowplaying/playerinterface/mpris2/mpris2_p.h 
04a3a76 
  plasma/generic/dataengines/nowplaying/playerinterface/org.kde.juk.player.xml 
bb6e18d 
  plasma/generic/dataengines/nowplaying/playerinterface/player.h 9c2e0fc 
  plasma/generic/dataengines/nowplaying/playerinterface/player.cpp b0f2680 
  plasma/generic/dataengines/nowplaying/playerinterface/playerfactory.h 27371f3 
  plasma/generic/dataengines/nowplaying/playerinterface/playerfactory.cpp 
53c1724 
  plasma/generic/dataengines/nowplaying/playerinterface/pollingwatcher.h 
34446da 
  plasma/generic/dataengines/nowplaying/playerinterface/pollingwatcher.cpp 
cf6bd99 
  plasma/generic/dataengines/nowplaying/playerinterface/xmms.h 8e58c73 
  plasma/generic/dataengines/nowplaying/playerinterface/xmms.cpp da3e9db 
  plasma/generic/dataengines/nowplaying/playerinterface/xmms_p.h 10a6e51 

Diff: http://git.reviewboard.kde.org/r/113993/diff/


Testing
---

CMake still runs successfully.


Thanks,

Alex Merry

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: [RFC] Moving Wishlist Items to Brainstorm

[Alex Merry]

On 25/05/13 07:58, Martin Graesslin wrote:
 For KWin I use the following template when we get a new feature request:

Since you wanted feedback from native speakers on the other one (which
was actually fine, by the way), I've tidied up this one (since the idea,
I guess, is to copy-paste it into reports).

Thank you for your feature suggestion. Unfortunately, bugzilla is not the
best place to discuss feature requests. We hardly have any users here, so we
don't get a useful evaluation of whether the feature is needed.

We recommend you use http://brainstorm.forum.kde.org to suggest
this idea. Users are able to up/down-vote the feature request so we can
get an idea of whether it's something our users wish to see and whether
it is
worth investing time in.

If the feature request is supported on brainstorm it will automatically be
recreated here in the bug tracker. Given that, I'm marking this feature
request as WONTFIX for now. Note that this has nothing to do with
whether we think
this is a good or bad feature request.

I'm very sorry that our software does not allow us to send users to
brainstorm
in the first place.



Alex
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 109857: Add the theme directory to the plasmoid directory structure

[Alex Merry]

On 03/05/13 13:03, Marco Martin wrote:
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/109857/
 
 
 On April 9th, 2013, 7:11 p.m. UTC, *Commit Hook* wrote:
 
 This review has been submitted with commit 
 24c26aefacff274ac52d773e39fbbfa3b458e213 by Alex Merry to branch master.
 
 Has this ever been ported to plasma-framework?

Now done: 33d41aea3dba1459de9db143a15dc20afea54f61

 and, it currently breaks unit tests

Sorry about that, and sorry for not replying sooner; I was out of the
country.  Thanks for fixing it.  I've made sure to fix the tests in the
forward port to plasma-framework.

Alex
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Review Request 109859: Allow plasmoids to provide fallbacks for themed images

[Alex Merry]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/109859/
---

Review request for Plasma.


Description
---

This adds an extra step to the lookup of SVG images created with the Svg() 
global function: after looking in images/ in the plasmoid and then in the 
desktop theme, it looks in theme/$DESKTOP_THEME_NAME and then theme/ in the 
plasmoid.

This allows plasmoid authors to add images that they want to allow theme 
authors to override.

Depends on RR 109857


Diffs
-

  plasma/scriptengines/javascript/plasmoid/themedsvg.cpp 
9de25a1bad3380ac964685b0516705d906a10592 

Diff: http://git.reviewboard.kde.org/r/109859/diff/


Testing
---

Checked with Now Playing that Svg(nocover) finds 
content/images/nocover.svgz and content/theme/nocover.svgz in the plasmoid, 
and that $KDEDIR/share/apps/desktoptheme/default/nocover.svgz overrides the 
latter.


Thanks,

Alex Merry

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Review Request 109857: Add the theme directory to the plasmoid directory structure

[Alex Merry]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/109857/
---

Review request for Plasma.


Description
---

This adds an extra step to the lookup of SVG images created with the Svg() 
global function: after looking in images/ in the plasmoid and then in the 
desktop theme, it looks in theme/$DESKTOP_THEME_NAME and then theme/ in the 
plasmoid.

This allows plasmoid authors to add images that they want to allow theme 
authors to override.


Diffs
-

  plasma/private/packages.cpp 1f346cc8af84ae10e4e7b878cf8c3a63c98c3509 

Diff: http://git.reviewboard.kde.org/r/109857/diff/


Testing
---

See RR 109859.


Thanks,

Alex Merry

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Now Listen plasmoid

[Alex Merry]

On 21/03/13 09:48, Aaron J. Seigo wrote:
 $PLASMOID_PACKAGE/images/file
 $SYSTEM_SVG_THEME_PATH/file
 $PLASMOID_PACKAGE/theme/$SYSTEM_SVG_THEME_NAME/file
 $PLASMOID_PACKAGE/theme/file

I've been trying to implement this, and have come across a hurdle:
Plasma::Theme does not have a way of getting hold of the fallback themes.

My current desktop theme is apparently .customized1 (I guess I changed
a setting at some point?).  However, all the image lookups will actually
fall back to the Air theme when I call Plasma::Theme::imagePath().  I
can't simulate this in ThemedSvg, though, as this information isn't
available.

So the options, I guess, are to either extend Theme (to either provide
the fallbacks or to have some sort of generic lookup mechanism
independent of the base directory) or to abandon the third part of this
lookup list.

Alex

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Now Listen plasmoid

[Alex Merry]

On 21/03/13 09:48, Aaron J. Seigo wrote:

On Thursday, March 21, 2013 00:00:52 you wrote:

PlasmaCore.SvgItem {
   svg: Svg(nocover)
}


yes, or Svg { source: nocover }


Does that really work?  When I did Svg { imagePath: nocover }, QML 
complained that Svg is not a type (it's a global function).



This seems to work, in that it finds content/images/nocover.svgz as
expected.  However, the effect is exactly the same as my original code,
which was

PlasmaCore.SvgItem {
   svg: plasmoid.file(images/nocover.svgz)
}


not really the same; this will never fallback to the theme


I meant that the effect in this instance (where the image is in the 
plasmoid) is the same.



would it be acceptable to you if we had a images/themed/ directory where you
could put such SVGs which would be the fallback to the theme?

this would give us the following search path:

$PLASMOID_PACKAGE/images/name
$SYSTEM_SVG_THEME_PATH/name
$PLASMOID_PACKAGE/images/themedname

this could get tripped up if the file path is themed/myimage.svg but that
seems like a stretch .. we could have a completely different directory, such as
theme/ ..

hell, we could even go completely crazy and allow having per-theme SVGs in
there even, e.g. a search path of:

$PLASMOID_PACKAGE/images/file
$SYSTEM_SVG_THEME_PATH/file
$PLASMOID_PACKAGE/theme/$SYSTEM_SVG_THEME_NAME/file
$PLASMOID_PACKAGE/theme/file

thoughts?

(this would all be very, very easy to add to ThemedSvg ..)


Any of these would be fine.  I think the separate directory approach 
would be safer, for the reason you mentioned.  I don't know whether 
anyone would really bother with the different theme files; it might be 
useful for some people, though.


Incidentally, if anyone feels the jewel case nocover image for the Now 
Playing plasmoid could be improved, do let me know.  I totally just 
stole it from Amarok, and I'm not sure a jewel case is really the right 
image to display there (especially as from 4.11 it will probably only be 
displayed when there is no media player; the media player logo is 
displayed when there is no album art).


Alex




___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Now Listen plasmoid

[Alex Merry]

On 20/03/13 13:00, Aaron J. Seigo wrote:

On Tuesday, March 19, 2013 23:44:37 Alex Merry wrote:

I investigated this, and there does not seem to be an easy way to do it
with QML plasmoids.  If you just ask for something from the theme, you
can't provide a default in the plasmoid.  To get something from the
plasmoid, you have to explicitly get it from there, and it can't then be
overridden in the theme.


If you use an SVG item, it will look first in the plasmoid, then in the theme.
in general, you should not need to request the path of the svg specifically
from a Theme item, but simply set the imagePath property appropriately.



Well, maybe that's how it should work, but the plasmoid lookup code 
for SVGs is kinda broken.  It only ever worked if the Plasma::SVG was a 
direct child of the Applet object (which you can't do in QML), and I'm 
pretty sure it doesn't work at all now (there are two bits of code that 
do the theme lookup, and the one that actually gets called first - via 
elementRect, IIRC - doesn't look in the plasmoid).


I had a look at fixing this, but couldn't determine a sensible way to 
find out what the plasmoid was from Plasma::SVG in the case of QML 
plasmoids.  Simply walking up the QObject parent tree doesn't work.


As a result, I end up using plasmoid.file to get the correct path to the 
SVG file.  I don't *think* that looks at the theme, though.  If it does, 
it doesn't appear to allow the theme to override the plasmoid file.


Alex
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Now Listen plasmoid

[Alex Merry]

On 19/03/13 23:38, Malcer wrote:
 Hi Alex! How are you? I hope well. :)
 
 I'm going to the point: I'm the creator of Caledonia, a plasma theme, and I 
 like your plasmoid... but exist a problem.
 
 Since a few versions, I don't succeed to change the CD case in the plasmoid 
 if 
 the song don't have mask (the album artwork, I mean). Before this, the CD 
 case 
 could be changed in plasma themes, in concrete with a SVG file called 
 nocover, in the folder widgets  nowplaying. 
 
 Now the plasmoid is QML, if I'm not mistaked, and I want to change the CD 
 case 
 with the plasma theme, as the old Now playing plasmoid.
 
 Is this possible? I'm wrong about how to change this piece of art? Can you 
 help me to this (or enable this possibility in a next release)?

I investigated this, and there does not seem to be an easy way to do it
with QML plasmoids.  If you just ask for something from the theme, you
can't provide a default in the plasmoid.  To get something from the
plasmoid, you have to explicitly get it from there, and it can't then be
overridden in the theme.

I'm CCing the plasma-devel list; does anyone know if there's a good
solution for this?

Alex

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Developing plasmoids with QML

[Alex Merry]

On 02/02/13 14:40, Вика wrote:
 
 Hello. Trying to make my first QML-plasmoid I've faced with some
 problems. I haven't found solutions in KDE tutorials and my googling
 gave no results. Is plasma-devel a right place to ask for help with that?

Yes, although it's probably worth trying #plasma on IRC
(http://userbase.kde.org/IRC_Channels/en) first; it's generally easier
to give people help when you can talk to them in real time.

Alex

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Images in packages

[Alex Merry]

Plasma::Svg's image search doesn't appear to be able to find images in a
plasmoid's package.

There is code in Plasma::SvgPrivate::createRenderer that should attempt
to find such an image, but (a) it is never called (because
Plasma::SvgPrivate::elementRect is called first, and sets themeFailed)
and (b) even if it were, it wouldn't work for QML applets where the
Plasma.Svg item doesn't have an Applet parent (or even ancestor).

Now, for a plain Plasma.Svg item in QML, you can just call
plasmoid.file(images, something.svgz) explicitly, but if you are
using it via PlasmaCore.IconItem, trying to do the same results in some
odd scaling behaviour and an inability to use named SVG elements,
because IconItem ends up using KIcon internally, rather than Plasma.Svg
(because you passed in an explicit file).

Is there any (sensible) way to fix this?  Is this even intended to work?

Alex
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Thoughts about a better Quality Management process for Plasma

[Alex Merry]

On 14/01/13 10:22, Martin Gräßlin wrote:

I think this would also be interesting for Plasma:
* which Plasmoids are running
* which containments are present
* what are the settings of each Plasmoid


Note that there are potential privacy issues here. Unlike KWin, there is 
a fair chance that plasmoid configurations may contain personal 
information - particularly if there are sticky notes, for example.


Alex

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Thoughts about a better Quality Management process for Plasma

[Alex Merry]

On 14/01/13 15:21, Alex Fiestas wrote:

On Sunday 13 January 2013 15:50:25 Martin Gräßlin wrote:

*Every commit should be referenced to a bug*
What is the motivation for a commit? It's either a bug fix or it is a new
feature/improvement. If it's a bug it's clear that there has to be a bug
report for it (out of experience: there is always a bug, if not: create it).
If it's a feature, it should also have a feature request in the tracker.
Create it yourself if you need to. Sounds beaurocratic, but it comes quite
handy as it allows to generate changelogs from it, allows people to easily
test new features.Martin


In case of big features, I guess we could add the BUG in the commit made for
the merge right? Or you want exactly one commit for feature?


Or you just use CCBUG every commit in the series, except for the merge 
commit which you use BUG for (or use BUG in the final commit that 
completes the feature).


Alex

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Thoughts about a better Quality Management process for Plasma

[Alex Merry]

On 14/01/13 15:24, Aaron J. Seigo wrote:

* it means adopting bugzilla as our One True Workflow tool. that doesn't just
sound beaurocratic, it is beaurocratic. if someone appears with a patch fixing
some bug or implementing some feature that isn't in bugzilla, do we first send
them to create one before accepting it? ugh.


It's possibly worth noting that for minor releases, KDE-SC changelogs 
are already generated from bugzilla (well, actually, they just link to a 
bugzilla report).  So if you want your bugfix to appear there, you have 
to create a bug report (if it doesn't already exist) and use the 
fixed-in field.


Alex

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Moving screen locking away from krunner

[Alex Merry]

On 03/12/12 14:42, Davide Bettio wrote:
 Nice, I missed that change. Is it possible to backport any fix for
 plasma-netbook?

Hacky fix for old versions of plasma-netbook:
http://randomguy3.wordpress.com/2011/09/04/screen-locking-with-the-plasma-netbook-interface/

Alex

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Add fade effect on the nowplaying text cycling

[Alex Merry]

 On Aug. 21, 2012, 3:54 p.m., Commit Hook wrote:
  This review has been submitted with commit 
  48ccff55dabc7ad9a173ee6b50d955c85e42aa27 by Martin Klapetek to branch 
  KDE/4.9.

I don't think this should really have gone into 4.9, as it is a feature, rather 
than a bug fix.


- Alex


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/106095/#review17810
---


On Aug. 20, 2012, 11:13 a.m., Martin Klapetek wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/106095/
 ---
 
 (Updated Aug. 20, 2012, 11:13 a.m.)
 
 
 Review request for Plasma.
 
 
 Description
 ---
 
 When the nowplaying is in panel, it cycles through title, artist and album in 
 one Label. This adds a simple, decent fade effect between the changes.
 
 
 Diffs
 -
 
   applets/nowplaying/package/contents/ui/MetadataLine.qml b793464 
 
 Diff: http://git.reviewboard.kde.org/r/106095/diff/
 
 
 Testing
 ---
 
 In plasmoidviewer.
 
 
 Thanks,
 
 Martin Klapetek
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Plasma::Service API addition

[Alex Merry]

On 13/05/12 14:48, Aaron J. Seigo wrote:

On Friday, May 11, 2012 17:29:44 Alex Merry wrote:

It's probably worth considering the interaction between Plasma::Service
and declarative applets generally.  I ran into issues with keeping
around an instance of Plasma::Service (I tried to keep it as a QtObject
property on a QML item, which didn't work), and eventually managed to
get it working by storing it as a variable in a separate .js file (which
I remembered doing in another QML project of mine).


putting it in a variant roperty didn't work?


I'm not sure I actually tried that.


I think having a DataSource equivalent for Plasma::Service would be
good, and allowing things like

Button {
text: Open
enabled: mprisService.commands.openUri.enabled
onClicked: mprisService.commands.openUri({url = uriEdit.text});
}

or something along those lines.


that would be nice, with some caveats: Service is not a push API, so the
enabled example above would imply polling. really, that's more appropriate to
a data engine. the onClicked example makes sense, however.


The enabled thing is meant to be an equivalent to associateWidget. 
That part of Service does behave like a push API (when available; for 
example, mpris2 changes the enabled states of commands when it is 
notified of a capability change by the media player).


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Plasma::Service API addition

[Alex Merry]

So I was about to commit the new nowplaying QML plasmoid to
kdeplasma-addons, when I realised that it depends on the API additions
in https://git.reviewboard.kde.org/r/104760/, which doesn't appear to
have been looked at.

Now we're actually past the soft feature freeze, and while I put the
mpris2 dataengine and nowplaying plasmoid in the feature list, the new
API isn't in the feature list.

What's the best thing to do here?

Alex
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: mpris2 engine and nowplaying QML applet

[Alex Merry]

On 10/05/12 12:13, Alex Merry wrote:
 Thanks.  I've applied all your suggestions to the scratch repo.  I'll
 probably commit to kde-workspace and kdeplasma-addons tomorrow morning,
 I imagine (or whenever I'm next on my home computer).

Done.

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Plasma::Service API addition

[Alex Merry]

On 11/05/12 12:18, Aaron J. Seigo wrote:

On Friday, May 11, 2012 11:59:59 Alex Merry wrote:

What's the best thing to do here?


i just reviewed the change; it can go in imho. it's a short term fix, though,
as we need to chnage this for libplasma2...


It's probably worth considering the interaction between Plasma::Service 
and declarative applets generally.  I ran into issues with keeping 
around an instance of Plasma::Service (I tried to keep it as a QtObject 
property on a QML item, which didn't work), and eventually managed to 
get it working by storing it as a variable in a separate .js file (which 
I remembered doing in another QML project of mine).


I think having a DataSource equivalent for Plasma::Service would be 
good, and allowing things like


Button {
text: Open
enabled: mprisService.commands.openUri.enabled
onClicked: mprisService.commands.openUri({url = uriEdit.text});
}

or something along those lines.

Alex
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: mpris2 engine and nowplaying QML applet

[Alex Merry]

On 10/05/12 11:51, David Edmundson wrote:

Only thing that was important was:

void MultiplexedService::activePlayerChanged(PlayerContainer *container)
{
-delete m_control.data();
+m_control.data()-deleteLater()


Other considerations:
[snip]


Thanks.  I've applied all your suggestions to the scratch repo.  I'll 
probably commit to kde-workspace and kdeplasma-addons tomorrow morning, 
I imagine (or whenever I'm next on my home computer).


Alex
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: mpris2 engine and nowplaying QML applet

[Alex Merry]

On 05/05/12 19:00, David Edmundson wrote:
 Has anyone reviewed the Mpris-dataengine? If not I'll do that this evening.

Eike Hein's done some testing of it, but hasn't actually reveiwed the
code (and I'm not sure how much he follows plasma development anyway).

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: mpris2 engine and nowplaying QML applet

[Alex Merry]

On 03/05/12 22:18, David Edmundson wrote:
 Now Playing QML Review:
 all code should be in - contents/ui, not at the top level dir.

Done

 Do you really want it set to keep-aspect ratio? It's not a great
 default aspect ratio anyway.

Ah, I'd forgotten that it's necessary to explicitly turn off keeping the
aspect ratio.  Fixed.

 Controls can overlap with the bottom row of text when resized. It
 may be better to set it so that the gap for the controls is always
 present.

This was a deliberate decision; most of the time the controls are not
needed, so reserving a space for them just prevents the user from saving
space for no real reason.

 MetadataPanel.qml
 albumLabel:
 Don't set the opacity on a font to change the colour. It works for
 black on white, but easily has the potential to be completely
 unreadable on some colour palettes/plasma themes. You're also
 implicitly using a new colour that the user hasn't specified, if
 everyone did this we would end up with so many inconsistent colours as
 every author chooses their own opacity. Specify it directly using
 something from the user's theme. (inactive would be a similar colour
 on a standard oxygen setup)
 
 Though if we were following other plasma applets such as the Power
 Applet (so a convention that is setting in, though we really need to
 write some HIG) the convention would be
 by: bThe Beatles/b all in the regular colour.
 
  (note also the additional colon in the label)

I went for setting font.weight to light, instead.  The reason I didn't
go for increasing the font weight of the fields themselves is that most
of the time those labels are unnecessary, as the user will know which
field is which from knowledge of their own music collection.  It's
really just a hint for when it's ambiguous.

I also think that including the colon looks significantly worse than
omitting it.  In English, at least, it makes the whole text read like a
sentence, but the arrangement allows for it to be read a line-at-a-time
as well (which may be the only sensible way to read it in other
languages).  From a translating POV, Amarok's Current Track applet (on
its internal contextual view) follows the same pattern (in fact, I
copied the idea from there).

 Controls.qml / PlayPauseButton.qml
 why do you have a singleshot timer to associate controls? I
 imagine it's a work around for something not being ready, but it needs
 documenting for when the next guy comes along and simplifies(breaks)
 it.

Ah, that was entirely unnecessary, in fact, and was a relic from before
I figured out that it is possible to keep around a Plasma::Service in a
separate .js file.  Removed now.

Alex

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: mpris2 engine and nowplaying QML applet

[Alex Merry]

On 29/04/12 13:46, David Edmundson wrote:

Could you make sure to update this wiki page on QML porting progress:
http://community.kde.org/Plasma/PlasmoidScripting#Porting_Plasmoids_to_QML
Don't want anyone duplicating your work by mistake.


Done.  Thanks for the heads up.
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: mpris2 engine and nowplaying QML applet

[Alex Merry]

On 27/04/12 23:52, Alex Merry wrote:

This is following up my previous email about the mpris2 engine, and
asking for a formal review.

I've lost track of how the review procedure has changed since the move
to git, and I can't find documentation of it anywhere, so I apologise if
I screw this up.


Since my previous email got a generally favourable response, I'll wait 
two weeks (11th May) and, if there are no objections by then, I'll put 
mpris2 in kde-workspace, replace nowplaying in kdeplasma-addons and put 
a kWarning() in the nowplaying engine (in kde-workspace) saying that it 
is deprecated and mpris2 should be used instead.


Alex


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Review Request: Allow QML items to be associated with operations in Plasma::Service

[Alex Merry]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104760/
---

Review request for Plasma.


Description
---

Allow QML items to be associated with operations in Plasma::Service

QML items derive from QGraphicsObject, but not QGraphicsWidget.  We
actually only need QGraphicsObject (for the enabled property).

This allows calling (dis)associateWidget from QML (eg: in javascript
plasmoids) and passing in a QML item.


Diffs
-

  plasma/private/service_p.h 8afef0b6f1ca8bfa11b271fd3b29bc033a94c9a5 
  plasma/service.h 314909ba84702beb16a36208d1a3c058b1a20f77 
  plasma/service.cpp 3846e817d013cf26e2dcf57c5fd86783f061bb13 

Diff: http://git.reviewboard.kde.org/r/104760/diff/


Testing
---

Tested with a QML plasmoid in plasmoidviewer.


Thanks,

Alex Merry

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Allow QML items to be associated with operations in Plasma::Service

[Alex Merry]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104760/
---

(Updated April 27, 2012, 8:47 p.m.)


Review request for Plasma.


Changes
---

Rename the new methods to associateItem/disassociateItem, since otherwise the 
scriptengine can't disambiguate when passed a QGraphicsWidget.


Description (updated)
---

Allow QML items to be associated with operations in Plasma::Service

QML items derive from QGraphicsObject, but not QGraphicsWidget.  We
actually only need QGraphicsObject (for the enabled property).

This allows QML items to be associated with operations (eg: in declarative 
applets).

The new methods are called (dis)associateItem to prevent disambiguation issues.


Diffs (updated)
-

  plasma/private/service_p.h 8afef0b6f1ca8bfa11b271fd3b29bc033a94c9a5 
  plasma/service.h 314909ba84702beb16a36208d1a3c058b1a20f77 
  plasma/service.cpp 3846e817d013cf26e2dcf57c5fd86783f061bb13 

Diff: http://git.reviewboard.kde.org/r/104760/diff/


Testing
---

Tested with a QML plasmoid in plasmoidviewer.


Thanks,

Alex Merry

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

mpris2 engine and nowplaying QML applet

[Alex Merry]

This is following up my previous email about the mpris2 engine, and
asking for a formal review.

I've lost track of how the review procedure has changed since the move
to git, and I can't find documentation of it anywhere, so I apologise if
I screw this up.

The mpris2 engine, which I think should go in
kde-workspace/plasma/generic/dataengines (as per the previous thread
about it) is at
git://anongit.kde.org:scratch/alexmerry/mpris2-dataengine
It deprecates the nowplaying engine, which should be removed in 5.0.

I've rewritten the nowplaying applet in QML (with many improvements,
such as looking nice and behaving well in vertical and horizontal form
factors), and that should replace the existing nowplaying applet (of
which I am the maintainer) in kdeplasma-addons
git://anongit.kde.org:scratch/alexmerry/nowplaying-qml

Alex
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: mpris2 engine and nowplaying QML applet

[Alex Merry]

On 27/04/12 23:52, Alex Merry wrote:
 I've rewritten the nowplaying applet in QML (with many improvements,
 such as looking nice and behaving well in vertical and horizontal form
 factors), and that should replace the existing nowplaying applet (of
 which I am the maintainer) in kdeplasma-addons
 git://anongit.kde.org:scratch/alexmerry/nowplaying-qml

I forgot to mention: this depends on review request 104760
(https://git.reviewboard.kde.org/r/104760/)

Alex
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

declarative plasmoids and services

[Alex Merry]

While developing the nowplaying QML plasmoid (see previous email), I ran
into problems using services where
service.startOperationCall(service.operationDescription(Play))
would fail (Play takes no arguments), because startOperationCall would
get an invalid KConfigGroup (with no name).

It turned out that this was because I was storing the service in a
QtObject property, and some sort of typing information about functions
was being lost.  The solution was to always call serviceForSource() at
the point where I needed to use the service.

However, the design pattern for services is to always create a new one,
so we've either got a memory leak (if they don't go away), or
setAssociatedWidget() isn't going to work (if they do).

Brief testing suggests that setAssociated[Widget/Item]() works (subject
to https://git.reviewboard.kde.org/r/104760/).  That may indicate the
leak scenario, or I may have just been (un)lucky with garbage collection.

Is this something that needs to be dealt with?  Perhaps in libplasma2?

Alex
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

MPRIS2 dataengine

[Alex Merry]

So, the nowplaying dataengine sucks, and I've basically given up
maintaining it.  It's broken by design, causes lots of wakeups and
context switches etc etc etc.  Also, trying to support half a dozen
different ways of talking to a media player is crazy.

However, MPRIS2 [0] provides a solution.  All the kids are doing it
these days: Unity and Konversation, to name two projects, are
exclusively talking to MPRIS2-capable media players.  Amarok and VLC
have support for it, JuK and dragon will have support in 4.9, many other
media players either support it or are having patches contributed.

I wrote a mpris2 dataengine [1] that implements all the core parts of
the spec (including control via services).  It's fully asynchronous (no
more freezing plasma while talking over the bus).  It doesn't require
any polling, even for track position information.  It's fully tested in
plasma engine explorer.  It doesn't have widgets yet, but I plan to port
the nowplaying widget to it (or, more likely, write a nowplaying
replacement in Plasma Quick).

The question is: what should happen with this?  I'd like it to replace
the horrible, broken mess that is nowplaying completely, ideally.
However, there are presumably things using nowplaying.  I can't really
do a compatibility layer (otherwise you losing the nice no-polling
property, and also the time units clash).

Eike Hein suggested that mpris2 should go into kde-workspace, next to
nowplaying, to allow widget authors to port away from nowplaying.  (The
trouble with putting mpris2 in addons is that there would be an
incentive - ubiquity - for widget authors to continue to use
nowplaying).  In 5.0, nowplaying can be ditched, and mpris2 can move to
addons, if that's a more sensible place for it long-term.

Thoughts?

Alex



[0]: http://www.freedesktop.org/wiki/Specifications/mpris-spec
[1]: git://anongit.kde.org:scratch/alexmerry/mpris2-dataengine
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Review Request: Move the Lock Session shortcut to kwin from krunner

[Alex Merry]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102261/
---

Review request for kwin, Plasma, Aaron J. Seigo, Martin Gräßlin, and Farhad 
Hedayati Fard.


Summary
---

In the farhad_hf/lockscreen branch (that moves screen locking functionality 
from krunner to kwin), the shortcut for locking the screen (Ctrl+Alt+L by 
default) remains in KRunner, which will not necessarily be running (when plasma 
is in netbook mode, for example).  This moves the shortcut registration to 
kwin (including a necessary hack for unregistering krunner's claim on it).

(See also https://git.reviewboard.kde.org/r/101943/ ).

I can also merge latest master into the branch and/or add in a 
KWIN_BUILD_SCREENSAVER cmake option (as in the original review request) if you 
(Martin/Farhad) want.


Diffs
-

  krunner/krunnerapp.cpp 55ac5ca 
  kwin/screenlocker/screensaver/saverengine.h 3384d4a 
  kwin/screenlocker/screensaver/saverengine.cpp 6c15be6 
  kwin/useractions.cpp 44685b9 

Diff: http://git.reviewboard.kde.org/r/102261/diff


Testing
---


Thanks,

Alex

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Move screensaver and locking functionality to kwin from krunner

[Alex Merry]

 On July 15, 2011, 3:28 p.m., Martin Gräßlin wrote:
  The code from the SoK project is now in an own branch: 
  https://projects.kde.org/projects/kde/kdebase/kde-workspace/repository/revisions/2780fd91810bad353ac33422ce4b3eab291c4b47
  
  Comparing the two I prefer the SoK commit as it doesn't add files to kwin's 
  root directory (which I don't like any more and get's currently more and 
  more changed)
 
 Alex Merry wrote:
 The issue with the SoK version is that KRunner still maintains the 
 keyboard shortcut for locking the screen (Ctrl+Alt+L by default), and this 
 means that the shortcut won't work when krunner is not running (such as when 
 plasma is in netbook mode).
 
 The code I put in useractions.cpp should be transferred to that branch, I 
 think.
 
 Martin Gräßlin wrote:
 Yes it should be transferred, but AFAIK it is not enough to just transfer 
 the code. There is more magic required.
 
 
 As a matter of fact I don't want it in useractions.cpp, if at all it 
 would have to go to kwinbindings.cpp, but I would more like to have it 
 completely in the context of screenlocker (Similar to what we just did to 
 tabbox key handling).

OK, I've created another RR against that branch, following the same pattern as 
tabbox: https://git.reviewboard.kde.org/r/102261/

I'll discard this one.


- Alex


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101943/#review4741
---


On July 13, 2011, 4:07 p.m., Alex Merry wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/101943/
 ---
 
 (Updated July 13, 2011, 4:07 p.m.)
 
 
 Review request for kwin, Plasma, Aaron J. Seigo, Martin Gräßlin, and Farhad 
 Hedayati Fard.
 
 
 Summary
 ---
 
 This transfers the screen locking and screensaver funcitonality wholesale 
 from krunner to kwin.  This has been OK'd in principle by the relevant 
 maintainers.
 
 Most of the code is simply moved untouched.  Points of note:
 
 I've introduced a KWIN_BUILD_SCREENSAVER option, to match the other kwin 
 build options, like KWIN_BUILD_TABBOX.  I disabled it for Plasma active, but 
 that may not be appropriate.
 
 I put the screensaver stuff (creating the SaverEngine object and setting up 
 the shortcut) in KWin::Workspace.  The shortcut stuff is actually in 
 useractions.cpp, but this implements methods in KWin::Workspace.
 
 I'm using the kglobalaccel D-Bus interface directly to steal the existing 
 shortcut from KRunner.  I'm doing it like this because the 
 KAction/KGlobalAccel interface is not rich enough to do this (probably wisely 
 - this isn't something apps should generally be doing).  The shortcut 
 deregistration happens in KWin rather than KRunner because I don't want to 
 rely on the order in which KWin and KRunner are started (or even on KRunner 
 being started at all).
 
 
 Diffs
 -
 
   CMakeLists.txt 89d97cd 
   kcontrol/screensaver/CMakeLists.txt e4dcc3a 
   krunner/CMakeLists.txt 4455847 
   krunner/README c8b9740 
   krunner/config-xautolock.h.cmake eadb0a6 
   krunner/dbus/org.freedesktop.ScreenSaver.xml 5efd943 
   krunner/dbus/org.kde.screensaver.xml e700b88 
   krunner/kcfg/kscreensaversettings.kcfg c8f76f3 
   krunner/kcfg/kscreensaversettings.kcfgc af9133d 
   krunner/krunnerapp.h 82db725 
   krunner/krunnerapp.cpp c143be5 
   krunner/lock/CMakeLists.txt cf9a67e 
   krunner/lock/autologout.h 0c44405 
   krunner/lock/autologout.cc c86e29a 
   krunner/lock/config-krunner-lock.h.cmake 7bfdfd6 
   krunner/lock/kscreenlocker.notifyrc 2955c5f 
   krunner/lock/lockdlg.h f25e55f 
   krunner/lock/lockdlg.cc 6367216 
   krunner/lock/lockprocess.h 8b6d9a8 
   krunner/lock/lockprocess.cc ecc632f 
   krunner/lock/main.h 8a60353 
   krunner/lock/main.cc 9f1c9b8 
   krunner/main.cpp 84a547b 
   krunner/screensaver/saverengine.h 3384d4a 
   krunner/screensaver/saverengine.cpp 6c15be6 
   krunner/screensaver/xautolock.h 3db3233 
   krunner/screensaver/xautolock.cpp 7124215 
   krunner/screensaver/xautolock_c.h 3b82f5c 
   krunner/screensaver/xautolock_diy.c b9df2f8 
   krunner/screensaver/xautolock_engine.c d6d0cf5 
   kscreenlocker/CMakeLists.txt PRE-CREATION 
   kscreenlocker/autologout.h PRE-CREATION 
   kscreenlocker/autologout.cc PRE-CREATION 
   kscreenlocker/config-kscreenlocker.h.cmake PRE-CREATION 
   kscreenlocker/kscreenlocker.notifyrc PRE-CREATION 
   kscreenlocker/lockdlg.h PRE-CREATION 
   kscreenlocker/lockdlg.cc PRE-CREATION 
   kscreenlocker/lockprocess.h PRE-CREATION 
   kscreenlocker/lockprocess.cc PRE-CREATION 
   kscreenlocker/main.h PRE-CREATION 
   kscreenlocker/main.cc PRE-CREATION 
   kwin/CMakeLists.txt 7d6ea52 
   kwin/config-kwin.h.cmake a291859 
   kwin/config-xautolock.h.cmake PRE

Re: Review Request: Move screensaver and locking functionality to kwin from krunner

[Alex Merry]

 On July 15, 2011, 3:28 p.m., Martin Gräßlin wrote:
  The code from the SoK project is now in an own branch: 
  https://projects.kde.org/projects/kde/kdebase/kde-workspace/repository/revisions/2780fd91810bad353ac33422ce4b3eab291c4b47
  
  Comparing the two I prefer the SoK commit as it doesn't add files to kwin's 
  root directory (which I don't like any more and get's currently more and 
  more changed)

The issue with the SoK version is that KRunner still maintains the keyboard 
shortcut for locking the screen (Ctrl+Alt+L by default), and this means that 
the shortcut won't work when krunner is not running (such as when plasma is in 
netbook mode).

The code I put in useractions.cpp should be transferred to that branch, I think.


- Alex


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101943/#review4741
---


On July 13, 2011, 4:07 p.m., Alex Merry wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/101943/
 ---
 
 (Updated July 13, 2011, 4:07 p.m.)
 
 
 Review request for kwin, Plasma, Aaron J. Seigo, Martin Gräßlin, and Farhad 
 Hedayati Fard.
 
 
 Summary
 ---
 
 This transfers the screen locking and screensaver funcitonality wholesale 
 from krunner to kwin.  This has been OK'd in principle by the relevant 
 maintainers.
 
 Most of the code is simply moved untouched.  Points of note:
 
 I've introduced a KWIN_BUILD_SCREENSAVER option, to match the other kwin 
 build options, like KWIN_BUILD_TABBOX.  I disabled it for Plasma active, but 
 that may not be appropriate.
 
 I put the screensaver stuff (creating the SaverEngine object and setting up 
 the shortcut) in KWin::Workspace.  The shortcut stuff is actually in 
 useractions.cpp, but this implements methods in KWin::Workspace.
 
 I'm using the kglobalaccel D-Bus interface directly to steal the existing 
 shortcut from KRunner.  I'm doing it like this because the 
 KAction/KGlobalAccel interface is not rich enough to do this (probably wisely 
 - this isn't something apps should generally be doing).  The shortcut 
 deregistration happens in KWin rather than KRunner because I don't want to 
 rely on the order in which KWin and KRunner are started (or even on KRunner 
 being started at all).
 
 
 Diffs
 -
 
   CMakeLists.txt 89d97cd 
   kcontrol/screensaver/CMakeLists.txt e4dcc3a 
   krunner/CMakeLists.txt 4455847 
   krunner/README c8b9740 
   krunner/config-xautolock.h.cmake eadb0a6 
   krunner/dbus/org.freedesktop.ScreenSaver.xml 5efd943 
   krunner/dbus/org.kde.screensaver.xml e700b88 
   krunner/kcfg/kscreensaversettings.kcfg c8f76f3 
   krunner/kcfg/kscreensaversettings.kcfgc af9133d 
   krunner/krunnerapp.h 82db725 
   krunner/krunnerapp.cpp c143be5 
   krunner/lock/CMakeLists.txt cf9a67e 
   krunner/lock/autologout.h 0c44405 
   krunner/lock/autologout.cc c86e29a 
   krunner/lock/config-krunner-lock.h.cmake 7bfdfd6 
   krunner/lock/kscreenlocker.notifyrc 2955c5f 
   krunner/lock/lockdlg.h f25e55f 
   krunner/lock/lockdlg.cc 6367216 
   krunner/lock/lockprocess.h 8b6d9a8 
   krunner/lock/lockprocess.cc ecc632f 
   krunner/lock/main.h 8a60353 
   krunner/lock/main.cc 9f1c9b8 
   krunner/main.cpp 84a547b 
   krunner/screensaver/saverengine.h 3384d4a 
   krunner/screensaver/saverengine.cpp 6c15be6 
   krunner/screensaver/xautolock.h 3db3233 
   krunner/screensaver/xautolock.cpp 7124215 
   krunner/screensaver/xautolock_c.h 3b82f5c 
   krunner/screensaver/xautolock_diy.c b9df2f8 
   krunner/screensaver/xautolock_engine.c d6d0cf5 
   kscreenlocker/CMakeLists.txt PRE-CREATION 
   kscreenlocker/autologout.h PRE-CREATION 
   kscreenlocker/autologout.cc PRE-CREATION 
   kscreenlocker/config-kscreenlocker.h.cmake PRE-CREATION 
   kscreenlocker/kscreenlocker.notifyrc PRE-CREATION 
   kscreenlocker/lockdlg.h PRE-CREATION 
   kscreenlocker/lockdlg.cc PRE-CREATION 
   kscreenlocker/lockprocess.h PRE-CREATION 
   kscreenlocker/lockprocess.cc PRE-CREATION 
   kscreenlocker/main.h PRE-CREATION 
   kscreenlocker/main.cc PRE-CREATION 
   kwin/CMakeLists.txt 7d6ea52 
   kwin/config-kwin.h.cmake a291859 
   kwin/config-xautolock.h.cmake PRE-CREATION 
   kwin/kscreensaversettings.kcfg PRE-CREATION 
   kwin/kscreensaversettings.kcfgc PRE-CREATION 
   kwin/screensaver/org.freedesktop.ScreenSaver.xml PRE-CREATION 
   kwin/screensaver/org.kde.screensaver.xml PRE-CREATION 
   kwin/screensaver/saverengine.h PRE-CREATION 
   kwin/screensaver/saverengine.cpp PRE-CREATION 
   kwin/screensaver/xautolock.h PRE-CREATION 
   kwin/screensaver/xautolock.cpp PRE-CREATION 
   kwin/screensaver/xautolock_c.h PRE-CREATION 
   kwin/screensaver/xautolock_diy.c PRE-CREATION 
   kwin/screensaver/xautolock_engine.c PRE-CREATION 
   kwin/useractions.cpp

Review Request: Move screensaver and locking functionality to kwin from krunner

[Alex Merry]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101943/
---

Review request for kwin, Plasma, Aaron J. Seigo, and Martin Gräßlin.


Summary
---

This transfers the screen locking and screensaver funcitonality wholesale from 
krunner to kwin.  This has been OK'd in principle by the relevant maintainers.

Most of the code is simply moved untouched.  Points of note:

I've introduced a KWIN_BUILD_SCREENSAVER option, to match the other kwin build 
options, like KWIN_BUILD_TABBOX.  I disabled it for Plasma active, but that may 
not be appropriate.

I put the screensaver stuff (creating the SaverEngine object and setting up the 
shortcut) in KWin::Workspace.  The shortcut stuff is actually in 
useractions.cpp, but this implements methods in KWin::Workspace.

I'm using the kglobalaccel D-Bus interface directly to steal the existing 
shortcut from KRunner.  I'm doing it like this because the KAction/KGlobalAccel 
interface is not rich enough to do this (probably wisely - this isn't something 
apps should generally be doing).  The shortcut deregistration happens in KWin 
rather than KRunner because I don't want to rely on the order in which KWin and 
KRunner are started (or even on KRunner being started at all).


Diffs
-

  kcontrol/screensaver/CMakeLists.txt e4dcc3a 
  krunner/CMakeLists.txt 4455847 
  krunner/README c8b9740 
  krunner/config-xautolock.h.cmake eadb0a6 
  krunner/dbus/org.freedesktop.ScreenSaver.xml 5efd943 
  krunner/dbus/org.kde.screensaver.xml e700b88 
  krunner/kcfg/kscreensaversettings.kcfg c8f76f3 
  krunner/kcfg/kscreensaversettings.kcfgc af9133d 
  krunner/krunnerapp.h 82db725 
  krunner/krunnerapp.cpp c143be5 
  krunner/lock/CMakeLists.txt cf9a67e 
  krunner/lock/autologout.h 0c44405 
  krunner/lock/autologout.cc c86e29a 
  krunner/lock/config-krunner-lock.h.cmake 7bfdfd6 
  krunner/lock/kscreenlocker.notifyrc 2955c5f 
  krunner/lock/lockdlg.h f25e55f 
  krunner/lock/lockdlg.cc 6367216 
  krunner/lock/lockprocess.h 8b6d9a8 
  krunner/lock/lockprocess.cc ecc632f 
  krunner/lock/main.h 8a60353 
  krunner/lock/main.cc 9f1c9b8 
  krunner/main.cpp 84a547b 
  krunner/screensaver/saverengine.h 3384d4a 
  krunner/screensaver/saverengine.cpp 6c15be6 
  krunner/screensaver/xautolock.h 3db3233 
  krunner/screensaver/xautolock.cpp 7124215 
  krunner/screensaver/xautolock_c.h 3b82f5c 
  krunner/screensaver/xautolock_diy.c b9df2f8 
  krunner/screensaver/xautolock_engine.c d6d0cf5 
  kwin/CMakeLists.txt 7d6ea52 
  kwin/config-kwin.h.cmake a291859 
  kwin/useractions.cpp 387e499 
  kwin/workspace.h 66b9830 
  kwin/workspace.cpp 8cf5299 
  plasma/desktop/applets/kickoff/CMakeLists.txt bc5fa2e 
  plasma/generic/applets/lock_logout/CMakeLists.txt 8381d46 
  plasma/generic/containmentactions/contextmenu/CMakeLists.txt a1fc205 
  plasma/generic/runners/sessions/CMakeLists.txt 1b8292c 
  powerdevil/daemon/CMakeLists.txt bad3dae 

Diff: http://git.reviewboard.kde.org/r/101943/diff


Testing
---

Allowing the screensaver to activate (both terminating the screensaver before 
it locks and after, with lock after 60 seconds set).

Using the lock screen action from krunner.

Stealing a non-default shortcut from KRunner (set the krunner Lock Session 
shortcut to another sequence, and ran KWin; KWin successfully deregistered 
krunner's Lock Session shortcut and assigned the key sequence to its own Lock 
Session shortcut).

Running KWin when no existing Lock Session shortcuts had been defined (either 
for krunner or kwin).  KWin successfully registered its Lock Session shortcut 
with the default key sequence (this is what would happen with a fresh user 
account).


Thanks,

Alex

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Move screensaver and locking functionality to kwin from krunner

[Alex Merry]

 On July 13, 2011, 12:54 p.m., Aaron J. Seigo wrote:
  I disabled it for Plasma active, but that may not be appropriate.
  
  we still need screen locking in Active, so this probably isn't entirely 
  correct. what we probably want, however, is a replacement for the actual 
  lock process which isn't appropriate for Active. right now we have a QML 
  based thing that doesn't use passwords, and it would be very nice to use 
  that as a full screen app in replacement of the desktop-appropriate lock 
  app. this would also neatly remove the desktopy screen savers from Active, 
  and that's just fine.
  
  in the patch i don't see where the lock process file get added to.. just 
  lots and lots of code removals :) i assume this was a problem with the 
  post-review script or some such and that you used `git mv` to move the 
  files somewhere appropriate. i actulaly think the lock process should sit 
  in the top level of kde-workspace, not hidden under whatever app launches 
  it. and then we can provide both a password-based one for the desktop in 
  there as well as a touch focused one; perhaps as build options, even.
  
  one other thing that probably needs to be done: the screensaver/lock window 
  should be identifitied as such to the window manager, just as we now do 
  with the desktop's Dashboard windows. dasbhoard marking is done with a 
  simple:
  
  
  #ifdef Q_WS_X11
  XClassHint classHint;
  classHint.res_name = DASHBOARD_WIN_NAME;
  classHint.res_class = DASHBOARD_WIN_CLASS;
  XSetClassHint(QX11Info::display(), windowId, classHint);
  #endif
  
  the name and class are defined in the file as simply dashboard. we could 
  do the same with the lock window and then when it is showing KWin can do 
  clever things like not paint any other windows.
  
  i like this patch so far, however, other than a few comments that follow 
  inline. Martin also needs to comment on it, of course.

Yes, I moved the lock process to the kwin directory using git mv... clearly git 
diff didn't show it up properly.  I can move it up to the top directory, maybe 
as kscreenlocker, since that's the name of the executable.

The window hinting is a good idea, but should probably go in as a separate 
patch.

I'll upload a new version addressing the points below this evening.


 On July 13, 2011, 12:54 p.m., Aaron J. Seigo wrote:
  kwin/workspace.cpp, lines 132-134
  http://git.reviewboard.kde.org/r/101943/diff/1/?file=26943#file26943line132
 
  this is reduendant to line 236 and doesn't provide any useful 
  initialization. the new should either be moved here, this line have a 0 
  added to it, or be removed.

Oh, that was supposed to have a 0 in it, in keeping with the other initializers 
in that class.


 On July 13, 2011, 12:54 p.m., Aaron J. Seigo wrote:
  kwin/workspace.cpp, line 236
  http://git.reviewboard.kde.org/r/101943/diff/1/?file=26943#file26943line236
 
  should check KWIN_BUILD_SCREENSAVER?

Yes.


 On July 13, 2011, 12:54 p.m., Aaron J. Seigo wrote:
  kwin/workspace.cpp, line 516
  http://git.reviewboard.kde.org/r/101943/diff/1/?file=26943#file26943line516
 
  should check KWIN_BUILD_SCREENSAVER?

Yes.


- Alex


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101943/#review4680
---


On July 13, 2011, 11:28 a.m., Alex Merry wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/101943/
 ---
 
 (Updated July 13, 2011, 11:28 a.m.)
 
 
 Review request for kwin, Plasma, Aaron J. Seigo, and Martin Gräßlin.
 
 
 Summary
 ---
 
 This transfers the screen locking and screensaver funcitonality wholesale 
 from krunner to kwin.  This has been OK'd in principle by the relevant 
 maintainers.
 
 Most of the code is simply moved untouched.  Points of note:
 
 I've introduced a KWIN_BUILD_SCREENSAVER option, to match the other kwin 
 build options, like KWIN_BUILD_TABBOX.  I disabled it for Plasma active, but 
 that may not be appropriate.
 
 I put the screensaver stuff (creating the SaverEngine object and setting up 
 the shortcut) in KWin::Workspace.  The shortcut stuff is actually in 
 useractions.cpp, but this implements methods in KWin::Workspace.
 
 I'm using the kglobalaccel D-Bus interface directly to steal the existing 
 shortcut from KRunner.  I'm doing it like this because the 
 KAction/KGlobalAccel interface is not rich enough to do this (probably wisely 
 - this isn't something apps should generally be doing).  The shortcut 
 deregistration happens in KWin rather than KRunner because I don't want to 
 rely on the order in which KWin and KRunner are started (or even on KRunner 
 being started at all).
 
 
 Diffs
 -
 
   kcontrol/screensaver

Re: Review Request: Move screensaver and locking functionality to kwin from krunner

[Alex Merry]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101943/
---

(Updated July 13, 2011, 3:54 p.m.)


Review request for kwin, Plasma, Aaron J. Seigo, Martin Gräßlin, and Farhad 
Hedayati Fard.


Changes
---

Added Farhad.


Summary
---

This transfers the screen locking and screensaver funcitonality wholesale from 
krunner to kwin.  This has been OK'd in principle by the relevant maintainers.

Most of the code is simply moved untouched.  Points of note:

I've introduced a KWIN_BUILD_SCREENSAVER option, to match the other kwin build 
options, like KWIN_BUILD_TABBOX.  I disabled it for Plasma active, but that may 
not be appropriate.

I put the screensaver stuff (creating the SaverEngine object and setting up the 
shortcut) in KWin::Workspace.  The shortcut stuff is actually in 
useractions.cpp, but this implements methods in KWin::Workspace.

I'm using the kglobalaccel D-Bus interface directly to steal the existing 
shortcut from KRunner.  I'm doing it like this because the KAction/KGlobalAccel 
interface is not rich enough to do this (probably wisely - this isn't something 
apps should generally be doing).  The shortcut deregistration happens in KWin 
rather than KRunner because I don't want to rely on the order in which KWin and 
KRunner are started (or even on KRunner being started at all).


Diffs
-

  kcontrol/screensaver/CMakeLists.txt e4dcc3a 
  krunner/CMakeLists.txt 4455847 
  krunner/README c8b9740 
  krunner/config-xautolock.h.cmake eadb0a6 
  krunner/dbus/org.freedesktop.ScreenSaver.xml 5efd943 
  krunner/dbus/org.kde.screensaver.xml e700b88 
  krunner/kcfg/kscreensaversettings.kcfg c8f76f3 
  krunner/kcfg/kscreensaversettings.kcfgc af9133d 
  krunner/krunnerapp.h 82db725 
  krunner/krunnerapp.cpp c143be5 
  krunner/lock/CMakeLists.txt cf9a67e 
  krunner/lock/autologout.h 0c44405 
  krunner/lock/autologout.cc c86e29a 
  krunner/lock/config-krunner-lock.h.cmake 7bfdfd6 
  krunner/lock/kscreenlocker.notifyrc 2955c5f 
  krunner/lock/lockdlg.h f25e55f 
  krunner/lock/lockdlg.cc 6367216 
  krunner/lock/lockprocess.h 8b6d9a8 
  krunner/lock/lockprocess.cc ecc632f 
  krunner/lock/main.h 8a60353 
  krunner/lock/main.cc 9f1c9b8 
  krunner/main.cpp 84a547b 
  krunner/screensaver/saverengine.h 3384d4a 
  krunner/screensaver/saverengine.cpp 6c15be6 
  krunner/screensaver/xautolock.h 3db3233 
  krunner/screensaver/xautolock.cpp 7124215 
  krunner/screensaver/xautolock_c.h 3b82f5c 
  krunner/screensaver/xautolock_diy.c b9df2f8 
  krunner/screensaver/xautolock_engine.c d6d0cf5 
  kwin/CMakeLists.txt 7d6ea52 
  kwin/config-kwin.h.cmake a291859 
  kwin/useractions.cpp 387e499 
  kwin/workspace.h 66b9830 
  kwin/workspace.cpp 8cf5299 
  plasma/desktop/applets/kickoff/CMakeLists.txt bc5fa2e 
  plasma/generic/applets/lock_logout/CMakeLists.txt 8381d46 
  plasma/generic/containmentactions/contextmenu/CMakeLists.txt a1fc205 
  plasma/generic/runners/sessions/CMakeLists.txt 1b8292c 
  powerdevil/daemon/CMakeLists.txt bad3dae 

Diff: http://git.reviewboard.kde.org/r/101943/diff


Testing
---

Allowing the screensaver to activate (both terminating the screensaver before 
it locks and after, with lock after 60 seconds set).

Using the lock screen action from krunner.

Stealing a non-default shortcut from KRunner (set the krunner Lock Session 
shortcut to another sequence, and ran KWin; KWin successfully deregistered 
krunner's Lock Session shortcut and assigned the key sequence to its own Lock 
Session shortcut).

Running KWin when no existing Lock Session shortcuts had been defined (either 
for krunner or kwin).  KWin successfully registered its Lock Session shortcut 
with the default key sequence (this is what would happen with a fresh user 
account).


Thanks,

Alex

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Move screensaver and locking functionality to kwin from krunner

[Alex Merry]

 On July 13, 2011, 3:38 p.m., Martin Gräßlin wrote:
  before I do a proper review I have a few questions:
  
  1. How does this align with the work our Season of KDE student, Farhad, is 
  doing? To my knowledge he already did what this review request provides. 
  Could you please add him to CC to ensure that we are not duplication work.
  
  2. What are the further plans? Are you aware of the architecture and ideas 
  outlined in http://community.kde.org/KWin/Screenlocker ?

1. I wasn't aware of anyone else working on this - I can find no trace of what 
Farhad has done, so I don't know whether he has already started/finished what I 
just did.  I've added him to this RR, though.

2. I had none.  I wasn't aware of that, no, although this RR is the obvious 
first step for the stuff in that document.  I just turned up on #plasma 
complaining about the fact that screen locking doesn't work in netbook mode in 
Plasma (due to the lack of krunner), and aseigo metioned that the consensus was 
that locking support should be in kwin.  I volunteered to do it.


- Alex


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101943/#review4689
---


On July 13, 2011, 3:54 p.m., Alex Merry wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/101943/
 ---
 
 (Updated July 13, 2011, 3:54 p.m.)
 
 
 Review request for kwin, Plasma, Aaron J. Seigo, Martin Gräßlin, and Farhad 
 Hedayati Fard.
 
 
 Summary
 ---
 
 This transfers the screen locking and screensaver funcitonality wholesale 
 from krunner to kwin.  This has been OK'd in principle by the relevant 
 maintainers.
 
 Most of the code is simply moved untouched.  Points of note:
 
 I've introduced a KWIN_BUILD_SCREENSAVER option, to match the other kwin 
 build options, like KWIN_BUILD_TABBOX.  I disabled it for Plasma active, but 
 that may not be appropriate.
 
 I put the screensaver stuff (creating the SaverEngine object and setting up 
 the shortcut) in KWin::Workspace.  The shortcut stuff is actually in 
 useractions.cpp, but this implements methods in KWin::Workspace.
 
 I'm using the kglobalaccel D-Bus interface directly to steal the existing 
 shortcut from KRunner.  I'm doing it like this because the 
 KAction/KGlobalAccel interface is not rich enough to do this (probably wisely 
 - this isn't something apps should generally be doing).  The shortcut 
 deregistration happens in KWin rather than KRunner because I don't want to 
 rely on the order in which KWin and KRunner are started (or even on KRunner 
 being started at all).
 
 
 Diffs
 -
 
   kcontrol/screensaver/CMakeLists.txt e4dcc3a 
   krunner/CMakeLists.txt 4455847 
   krunner/README c8b9740 
   krunner/config-xautolock.h.cmake eadb0a6 
   krunner/dbus/org.freedesktop.ScreenSaver.xml 5efd943 
   krunner/dbus/org.kde.screensaver.xml e700b88 
   krunner/kcfg/kscreensaversettings.kcfg c8f76f3 
   krunner/kcfg/kscreensaversettings.kcfgc af9133d 
   krunner/krunnerapp.h 82db725 
   krunner/krunnerapp.cpp c143be5 
   krunner/lock/CMakeLists.txt cf9a67e 
   krunner/lock/autologout.h 0c44405 
   krunner/lock/autologout.cc c86e29a 
   krunner/lock/config-krunner-lock.h.cmake 7bfdfd6 
   krunner/lock/kscreenlocker.notifyrc 2955c5f 
   krunner/lock/lockdlg.h f25e55f 
   krunner/lock/lockdlg.cc 6367216 
   krunner/lock/lockprocess.h 8b6d9a8 
   krunner/lock/lockprocess.cc ecc632f 
   krunner/lock/main.h 8a60353 
   krunner/lock/main.cc 9f1c9b8 
   krunner/main.cpp 84a547b 
   krunner/screensaver/saverengine.h 3384d4a 
   krunner/screensaver/saverengine.cpp 6c15be6 
   krunner/screensaver/xautolock.h 3db3233 
   krunner/screensaver/xautolock.cpp 7124215 
   krunner/screensaver/xautolock_c.h 3b82f5c 
   krunner/screensaver/xautolock_diy.c b9df2f8 
   krunner/screensaver/xautolock_engine.c d6d0cf5 
   kwin/CMakeLists.txt 7d6ea52 
   kwin/config-kwin.h.cmake a291859 
   kwin/useractions.cpp 387e499 
   kwin/workspace.h 66b9830 
   kwin/workspace.cpp 8cf5299 
   plasma/desktop/applets/kickoff/CMakeLists.txt bc5fa2e 
   plasma/generic/applets/lock_logout/CMakeLists.txt 8381d46 
   plasma/generic/containmentactions/contextmenu/CMakeLists.txt a1fc205 
   plasma/generic/runners/sessions/CMakeLists.txt 1b8292c 
   powerdevil/daemon/CMakeLists.txt bad3dae 
 
 Diff: http://git.reviewboard.kde.org/r/101943/diff
 
 
 Testing
 ---
 
 Allowing the screensaver to activate (both terminating the screensaver before 
 it locks and after, with lock after 60 seconds set).
 
 Using the lock screen action from krunner.
 
 Stealing a non-default shortcut from KRunner (set the krunner Lock Session 
 shortcut to another sequence, and ran KWin; KWin successfully deregistered 
 krunner's Lock Session shortcut

Re: Review Request: Move screensaver and locking functionality to kwin from krunner

[Alex Merry]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101943/
---

(Updated July 13, 2011, 4:07 p.m.)


Review request for kwin, Plasma, Aaron J. Seigo, Martin Gräßlin, and Farhad 
Hedayati Fard.


Changes
---

Updated to address aseigo's comments.


Summary
---

This transfers the screen locking and screensaver funcitonality wholesale from 
krunner to kwin.  This has been OK'd in principle by the relevant maintainers.

Most of the code is simply moved untouched.  Points of note:

I've introduced a KWIN_BUILD_SCREENSAVER option, to match the other kwin build 
options, like KWIN_BUILD_TABBOX.  I disabled it for Plasma active, but that may 
not be appropriate.

I put the screensaver stuff (creating the SaverEngine object and setting up the 
shortcut) in KWin::Workspace.  The shortcut stuff is actually in 
useractions.cpp, but this implements methods in KWin::Workspace.

I'm using the kglobalaccel D-Bus interface directly to steal the existing 
shortcut from KRunner.  I'm doing it like this because the KAction/KGlobalAccel 
interface is not rich enough to do this (probably wisely - this isn't something 
apps should generally be doing).  The shortcut deregistration happens in KWin 
rather than KRunner because I don't want to rely on the order in which KWin and 
KRunner are started (or even on KRunner being started at all).


Diffs (updated)
-

  CMakeLists.txt 89d97cd 
  kcontrol/screensaver/CMakeLists.txt e4dcc3a 
  krunner/CMakeLists.txt 4455847 
  krunner/README c8b9740 
  krunner/config-xautolock.h.cmake eadb0a6 
  krunner/dbus/org.freedesktop.ScreenSaver.xml 5efd943 
  krunner/dbus/org.kde.screensaver.xml e700b88 
  krunner/kcfg/kscreensaversettings.kcfg c8f76f3 
  krunner/kcfg/kscreensaversettings.kcfgc af9133d 
  krunner/krunnerapp.h 82db725 
  krunner/krunnerapp.cpp c143be5 
  krunner/lock/CMakeLists.txt cf9a67e 
  krunner/lock/autologout.h 0c44405 
  krunner/lock/autologout.cc c86e29a 
  krunner/lock/config-krunner-lock.h.cmake 7bfdfd6 
  krunner/lock/kscreenlocker.notifyrc 2955c5f 
  krunner/lock/lockdlg.h f25e55f 
  krunner/lock/lockdlg.cc 6367216 
  krunner/lock/lockprocess.h 8b6d9a8 
  krunner/lock/lockprocess.cc ecc632f 
  krunner/lock/main.h 8a60353 
  krunner/lock/main.cc 9f1c9b8 
  krunner/main.cpp 84a547b 
  krunner/screensaver/saverengine.h 3384d4a 
  krunner/screensaver/saverengine.cpp 6c15be6 
  krunner/screensaver/xautolock.h 3db3233 
  krunner/screensaver/xautolock.cpp 7124215 
  krunner/screensaver/xautolock_c.h 3b82f5c 
  krunner/screensaver/xautolock_diy.c b9df2f8 
  krunner/screensaver/xautolock_engine.c d6d0cf5 
  kscreenlocker/CMakeLists.txt PRE-CREATION 
  kscreenlocker/autologout.h PRE-CREATION 
  kscreenlocker/autologout.cc PRE-CREATION 
  kscreenlocker/config-kscreenlocker.h.cmake PRE-CREATION 
  kscreenlocker/kscreenlocker.notifyrc PRE-CREATION 
  kscreenlocker/lockdlg.h PRE-CREATION 
  kscreenlocker/lockdlg.cc PRE-CREATION 
  kscreenlocker/lockprocess.h PRE-CREATION 
  kscreenlocker/lockprocess.cc PRE-CREATION 
  kscreenlocker/main.h PRE-CREATION 
  kscreenlocker/main.cc PRE-CREATION 
  kwin/CMakeLists.txt 7d6ea52 
  kwin/config-kwin.h.cmake a291859 
  kwin/config-xautolock.h.cmake PRE-CREATION 
  kwin/kscreensaversettings.kcfg PRE-CREATION 
  kwin/kscreensaversettings.kcfgc PRE-CREATION 
  kwin/screensaver/org.freedesktop.ScreenSaver.xml PRE-CREATION 
  kwin/screensaver/org.kde.screensaver.xml PRE-CREATION 
  kwin/screensaver/saverengine.h PRE-CREATION 
  kwin/screensaver/saverengine.cpp PRE-CREATION 
  kwin/screensaver/xautolock.h PRE-CREATION 
  kwin/screensaver/xautolock.cpp PRE-CREATION 
  kwin/screensaver/xautolock_c.h PRE-CREATION 
  kwin/screensaver/xautolock_diy.c PRE-CREATION 
  kwin/screensaver/xautolock_engine.c PRE-CREATION 
  kwin/useractions.cpp 387e499 
  kwin/workspace.h 66b9830 
  kwin/workspace.cpp 8cf5299 
  plasma/desktop/applets/kickoff/CMakeLists.txt bc5fa2e 
  plasma/generic/applets/lock_logout/CMakeLists.txt 8381d46 
  plasma/generic/containmentactions/contextmenu/CMakeLists.txt a1fc205 
  plasma/generic/runners/sessions/CMakeLists.txt 1b8292c 
  powerdevil/daemon/CMakeLists.txt bad3dae 

Diff: http://git.reviewboard.kde.org/r/101943/diff


Testing
---

Allowing the screensaver to activate (both terminating the screensaver before 
it locks and after, with lock after 60 seconds set).

Using the lock screen action from krunner.

Stealing a non-default shortcut from KRunner (set the krunner Lock Session 
shortcut to another sequence, and ran KWin; KWin successfully deregistered 
krunner's Lock Session shortcut and assigned the key sequence to its own Lock 
Session shortcut).

Running KWin when no existing Lock Session shortcuts had been defined (either 
for krunner or kwin).  KWin successfully registered its Lock Session shortcut 
with the default key sequence (this is what would 

Re: Review Request: Make custom colours work on the digital clock again

[Alex Merry]

---
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/5747/
---

(Updated 2010-11-04 17:57:08.651004)


Review request for Plasma.


Changes
---

Configuration dialog updated as per Aaron's suggestions.  No changes to 
rendering code.

I'm also not happy with option proliferation, but this at least is in line with 
the other font/styling options that are already in the dialog.


Summary
---

Since the shadow was introduced for the digital clock a few weeks ago, the 
custom colour setting has been ignored.  This re-enables it, and also allows 
the user to choose a shadow colour.

This changes the configuration dialog and introduces a new option, which is why 
I'm submitting it for review before committing.


Diffs (updated)
-

  /trunk/KDE/kdebase/workspace/plasma/generic/applets/digital-clock/clock.h 
1192604 
  /trunk/KDE/kdebase/workspace/plasma/generic/applets/digital-clock/clock.cpp 
1192604 
  
/trunk/KDE/kdebase/workspace/plasma/generic/applets/digital-clock/clockConfig.ui
 1192604 

Diff: http://svn.reviewboard.kde.org/r/5747/diff


Testing
---

Changing, saving and loading the settings worked in plasmoid-viewer.


Screenshots
---

The changed configuration
  http://svn.reviewboard.kde.org/r/5747/s/546/
Updated
  http://svn.reviewboard.kde.org/r/5747/s/549/


Thanks,

Alex

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Make custom colours work on the digital clock again

[Alex Merry]

---
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/5747/
---

(Updated 2010-11-04 02:07:24.050751)


Review request for Plasma.


Changes
---

Screenshot included.

I made disabling the shadow an option because that's the only way I can get 
anything that looks half decent on my desktop (I have the default panel at the 
top of the screen with Fresh Morning as my background - the panel is 
translucent, so the theme's default colours look really bad and are hard to 
read, but plain solid white looks fine.

The reason this is a combined feature/bug fix is because I couldn't figure out 
a sensible way to fix the bug without including the option to change the shadow 
(given that black text with a black shadow, for example, is not a sensible 
combination).  Well, other than removing the option to set the text colour 
altogether, but that would give me an ugly, hard-to-read clock.


Summary
---

Since the shadow was introduced for the digital clock a few weeks ago, the 
custom colour setting has been ignored.  This re-enables it, and also allows 
the user to choose a shadow colour.

This changes the configuration dialog and introduces a new option, which is why 
I'm submitting it for review before committing.


Diffs
-

  /trunk/KDE/kdebase/workspace/plasma/generic/applets/digital-clock/clock.h 
1191270 
  /trunk/KDE/kdebase/workspace/plasma/generic/applets/digital-clock/clock.cpp 
1191270 
  
/trunk/KDE/kdebase/workspace/plasma/generic/applets/digital-clock/clockConfig.ui
 1191270 

Diff: http://svn.reviewboard.kde.org/r/5747/diff


Testing
---

Changing, saving and loading the settings worked in plasmoid-viewer.


Screenshots
---

The changed configuration
  http://svn.reviewboard.kde.org/r/5747/s/546/


Thanks,

Alex

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel