Re: Review Request 118388: rename systemsettings binary to systemsettings5

2014-05-29 Thread Ben Cooksley

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


Code wise, this change looks fine. In terms of renaming the desktop files - i'm 
fine with changing the filenames, but please don't change the name of the 
application itself. Ideally the KF5 based system settings will still be able to 
set configuration details relevant for KDE 4 applications.

- Ben Cooksley


On May 28, 2014, 7:32 p.m., Hrvoje Senjan wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118388/
 ---
 
 (Updated May 28, 2014, 7:32 p.m.)
 
 
 Review request for Plasma and Ben Cooksley.
 
 
 Repository: systemsettings
 
 
 Description
 ---
 
 while workspace might not be targeted to co-exist with 4.x variant - 
 systemsettings should IMHO be able to co-exist. not only workspace components 
 are adjusting in there, and telling people to do kcmshell$notinstalledvariant 
 $wantedkcm is very user-unfriendly...
 one TODO if this gets a green light, is to rename desktop files, so people 
 know which variant they are opening.
 
 
 Diffs
 -
 
   app/systemsettings.desktop 5f27318 
   app/kdesystemsettings.desktop 946d498 
   app/CMakeLists.txt c45f7e7 
 
 Diff: https://git.reviewboard.kde.org/r/118388/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Hrvoje Senjan
 


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


Re: Review Request 118390: Powerdevil KCM Updates

2014-05-29 Thread Thomas Pfeiffer

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


Good improvements in general!
There are two changes which are not HIG-compliant:
- Control labels which are written next to the controls (checkboxes, spin boxes 
etc.) do not use Title Capitalization. See 
http://techbase.kde.org/Projects/Usability/HIG/Capitalization for details. 
Therefore most of the capitalization changes have to be reverted, I'm afraid.
- For grouping controls (as in the case of Battery Levels and Events) the HIG 
recommends using group boxes (see 
http://techbase.kde.org/Projects/Usability/HIG/GroupBox ) instead of simply 
spacing
Apart from those two things, the changes look fine to me!

- Thomas Pfeiffer


On May 28, 2014, 11:33 p.m., Sebastian Kügler wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118390/
 ---
 
 (Updated May 28, 2014, 11:33 p.m.)
 
 
 Review request for Plasma, Solid and KDE Usability.
 
 
 Repository: powerdevil
 
 
 Description
 ---
 
 This is a series of UI updates I've applied to the powerdevil KCMs. You can 
 find them as individual patch series in the sebas/kcmupdates branch.
 
 General:
 - Less icon usage, especially in the form layouts
 - Title casing throughout
 - Better HIG compliance (not 100%, but improved)
 - Energy Saving / Actions UI now scales with dialog
 - Proper usage of FormLayouts
 - A bunch of cleanups of dead code
 - Parenting fixes
 
 In Detail:
 
 * Improve Advanced Settings page
 
 - Use a QFormLayout, and do it properly
 - Fix up spacing and alignment
 - Remove icons before titles
 - Use Title Case for Labels
 - Shorter labels for better readability
 
 * Clean up brightness-OSD-related dead code
 
 
 * Make powerdevil actions layout stretch out horizontally
 
 
 * Compile-time connections in actionconfigwidget
 
 
 * widget and layout parenting fixes
 
 
 * No bold font, increased spacing instead
 
 This removes the bold fonts from the checkboxes, as that is non-standard
 in the HIG. In order to make it look a bit more structured between the
 sections, a bit of spacing is added.
 
 * Remove icons from actions
 
 The icons are really small and add more visual noise than being useful.
 
 * Streamline Comments of KCMs
 
 
 * Clean up dead code
 
 
 Diffs
 -
 
   kcmodule/profiles/profileEditPage.ui 
 dc2657943a2bb63f137ad11b77b254b3782f0407 
   kcmodule/profiles/powerdevilprofilesconfig.desktop 
 e95b908dfea30a4a15ccd8c89d4ceb36256e2185 
   kcmodule/profiles/EditPage.cpp a674ccbdd76ba682c7b7111f2f9d0b9123ffe8b8 
   kcmodule/global/powerdevilglobalconfig.desktop 
 1d4aa2238b2fa02dfa984f9163f133c941ff509e 
   kcmodule/global/generalPage.ui 2ce7cef98e30f872ea9233edc7882deea134fdd3 
   kcmodule/global/GeneralPage.cpp d025e429df7ca191bf56443b2bd30353a74993a9 
   kcmodule/common/actioneditwidget.cpp 
 4c67b4feeb2cff0df2e397f44ffba2926bbb7276 
   kcmodule/common/actionconfigwidget.cpp 
 2161c84b68e5fc3b56240a482ed77825ce4a5f03 
   kcmodule/activities/powerdevilactivitiesconfig.desktop 
 9ee81b55ec7f4685adb5d102cb21df81f17cf3e3 
   kcmodule/activities/activitypage.cpp 
 48e9c6c75f2ab95ce52c96b586475dbb4d6022fd 
   daemon/actions/dpms/powerdevildpmsactionconfig.cpp 
 4d96273697f11864716dae0204009d52264be99f 
   daemon/actions/bundled/runscriptconfig.cpp 
 c0d3adb6651320702bb6503e505b6ebdc06d6004 
   daemon/actions/bundled/keyboardbrightnesscontrolconfig.cpp 
 31772679560a256dcf9f66aefbb4cf02351bd5d3 
   daemon/actions/bundled/keyboardbrightnesscontrol.cpp 
 44dbcd87c06f5dce6d1ff10697837a60cdcc5f29 
   daemon/actions/bundled/dimdisplayconfig.cpp 
 f683935994e4a5a08fd7da6ceb95ba7b726ac708 
   daemon/actions/bundled/brightnesscontrolconfig.cpp 
 fea2a3ea7cb44c27496ea81f4fe35a8ccfde3acc 
 
 Diff: https://git.reviewboard.kde.org/r/118390/diff/
 
 
 Testing
 ---
 
 Tested in kcmshell5 and systemsettings, still fully functional.
 
 
 File Attachments
 
 
 Energy Saving page before
   
 https://git.reviewboard.kde.org/media/uploaded/files/2014/05/28/680a38ab-18d6-4343-86e8-6d6aeaf63032__powerdevil-kcm-profiles-before.png
 Energy Saving page after
   
 https://git.reviewboard.kde.org/media/uploaded/files/2014/05/28/30357866-df50-4c1a-afc2-63e3e565f55a__powerdevil-kcm-profiles-after.png
 Advanced page after
   
 https://git.reviewboard.kde.org/media/uploaded/files/2014/05/28/36401d04-da7a-4d30-8b6d-a64ccf3a7865__powerdevil-kcm-advanced-after.png
 Advanced page before
   
 https://git.reviewboard.kde.org/media/uploaded/files/2014/05/28/e06c29ef-c926-4ae9-b01b-f502c3cfd0a0__powerdevil-kcm-advanced-before.png
 
 
 Thanks,
 
 Sebastian Kügler
 


___
Plasma-devel mailing list

Re: Review Request 118390: Powerdevil KCM Updates

2014-05-29 Thread Kai Uwe Broulik

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


Thanks for looking into this! See my issues below, some of them are if you're 
already at it things.


File Attachment: Energy Saving page after - powerdevil-kcm-profiles-after.png
https://git.reviewboard.kde.org//r/118390/#fcomment222
Why remove the icons? They make the individual settings recognizable more 
quickly.


File Attachment: Advanced page after - powerdevil-kcm-advanced-after.png
https://git.reviewboard.kde.org//r/118390/#fcomment223
Indeed, GroupBoxes should be used here


File Attachment: Advanced page after - powerdevil-kcm-advanced-after.png
https://git.reviewboard.kde.org//r/118390/#fcomment224
That empty space looks weird


File Attachment: Advanced page after - powerdevil-kcm-advanced-after.png
https://git.reviewboard.kde.org//r/118390/#fcomment225
Those should have a colon at the end


daemon/actions/bundled/dimdisplayconfig.cpp
https://git.reviewboard.kde.org/r/118390/#comment40848

That should be come dpi independent


- Kai Uwe Broulik


On May 28, 2014, 11:33 p.m., Sebastian Kügler wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118390/
 ---
 
 (Updated May 28, 2014, 11:33 p.m.)
 
 
 Review request for Plasma, Solid and KDE Usability.
 
 
 Repository: powerdevil
 
 
 Description
 ---
 
 This is a series of UI updates I've applied to the powerdevil KCMs. You can 
 find them as individual patch series in the sebas/kcmupdates branch.
 
 General:
 - Less icon usage, especially in the form layouts
 - Title casing throughout
 - Better HIG compliance (not 100%, but improved)
 - Energy Saving / Actions UI now scales with dialog
 - Proper usage of FormLayouts
 - A bunch of cleanups of dead code
 - Parenting fixes
 
 In Detail:
 
 * Improve Advanced Settings page
 
 - Use a QFormLayout, and do it properly
 - Fix up spacing and alignment
 - Remove icons before titles
 - Use Title Case for Labels
 - Shorter labels for better readability
 
 * Clean up brightness-OSD-related dead code
 
 
 * Make powerdevil actions layout stretch out horizontally
 
 
 * Compile-time connections in actionconfigwidget
 
 
 * widget and layout parenting fixes
 
 
 * No bold font, increased spacing instead
 
 This removes the bold fonts from the checkboxes, as that is non-standard
 in the HIG. In order to make it look a bit more structured between the
 sections, a bit of spacing is added.
 
 * Remove icons from actions
 
 The icons are really small and add more visual noise than being useful.
 
 * Streamline Comments of KCMs
 
 
 * Clean up dead code
 
 
 Diffs
 -
 
   kcmodule/profiles/profileEditPage.ui 
 dc2657943a2bb63f137ad11b77b254b3782f0407 
   kcmodule/profiles/powerdevilprofilesconfig.desktop 
 e95b908dfea30a4a15ccd8c89d4ceb36256e2185 
   kcmodule/profiles/EditPage.cpp a674ccbdd76ba682c7b7111f2f9d0b9123ffe8b8 
   kcmodule/global/powerdevilglobalconfig.desktop 
 1d4aa2238b2fa02dfa984f9163f133c941ff509e 
   kcmodule/global/generalPage.ui 2ce7cef98e30f872ea9233edc7882deea134fdd3 
   kcmodule/global/GeneralPage.cpp d025e429df7ca191bf56443b2bd30353a74993a9 
   kcmodule/common/actioneditwidget.cpp 
 4c67b4feeb2cff0df2e397f44ffba2926bbb7276 
   kcmodule/common/actionconfigwidget.cpp 
 2161c84b68e5fc3b56240a482ed77825ce4a5f03 
   kcmodule/activities/powerdevilactivitiesconfig.desktop 
 9ee81b55ec7f4685adb5d102cb21df81f17cf3e3 
   kcmodule/activities/activitypage.cpp 
 48e9c6c75f2ab95ce52c96b586475dbb4d6022fd 
   daemon/actions/dpms/powerdevildpmsactionconfig.cpp 
 4d96273697f11864716dae0204009d52264be99f 
   daemon/actions/bundled/runscriptconfig.cpp 
 c0d3adb6651320702bb6503e505b6ebdc06d6004 
   daemon/actions/bundled/keyboardbrightnesscontrolconfig.cpp 
 31772679560a256dcf9f66aefbb4cf02351bd5d3 
   daemon/actions/bundled/keyboardbrightnesscontrol.cpp 
 44dbcd87c06f5dce6d1ff10697837a60cdcc5f29 
   daemon/actions/bundled/dimdisplayconfig.cpp 
 f683935994e4a5a08fd7da6ceb95ba7b726ac708 
   daemon/actions/bundled/brightnesscontrolconfig.cpp 
 fea2a3ea7cb44c27496ea81f4fe35a8ccfde3acc 
 
 Diff: https://git.reviewboard.kde.org/r/118390/diff/
 
 
 Testing
 ---
 
 Tested in kcmshell5 and systemsettings, still fully functional.
 
 
 File Attachments
 
 
 Energy Saving page before
   
 https://git.reviewboard.kde.org/media/uploaded/files/2014/05/28/680a38ab-18d6-4343-86e8-6d6aeaf63032__powerdevil-kcm-profiles-before.png
 Energy Saving page after
   
 https://git.reviewboard.kde.org/media/uploaded/files/2014/05/28/30357866-df50-4c1a-afc2-63e3e565f55a__powerdevil-kcm-profiles-after.png
 Advanced page after
   
 

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

2014-05-29 Thread 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: Ship with Aurorae and Qtcurve or not...

2014-05-29 Thread Marco Martin
On Thursday 15 May 2014 11:39:00 Jens Reuterberg wrote:
 Ok so after the feedback from the Beta Release an issue that we knew was
 coming have happened. Visuals being the most easily accessible bit of
 anything technical, people have reacted negatively to the lack of change.

just to give a shot on every and single options, i gave a try to modifying 
oxygen in order to make it look like breeze (therefore sharing all the things 
that it does that are not related to painting, like drag the window from 
anywhere)
this is the half done, half missing attempt (ignore the non changed elements)
http://wstaw.org/m/2014/05/29/plasma-desktophP1414.png

is pretty hacky, but *maybe* is possible to have in the end only a different 
stylehelper (and pixelmetrics/stylehints)
so could be something worth exploring in the future

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


Re: Review Request 118382: [klipper] Fix i18n

2014-05-29 Thread David Edmundson

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

Ship it!


I'm not very happy giving a ship it to something that is ported but completely 
untested. We end up with code we think works, but doesn't work properly; which 
is worse than not touching it at all.

It should be possible to download the x-test translations? Lets ask i18n if we 
need to.

- David Edmundson


On May 28, 2014, 3:22 p.m., Martin Gräßlin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118382/
 ---
 
 (Updated May 28, 2014, 3:22 p.m.)
 
 
 Review request for Plasma.
 
 
 Repository: plasma-workspace
 
 
 Description
 ---
 
 [klipper] Fix i18n
 
 
 Diffs
 -
 
   klipper/CMakeLists.txt 7a57b30b8b59a77c3d0702095dd72bb275d22e57 
   klipper/main.cpp 153a03160717431eaaa95e77903ab43de9b07fcb 
 
 Diff: https://git.reviewboard.kde.org/r/118382/diff/
 
 
 Testing
 ---
 
 I still don't know how to test i18n with kf5 :-)
 
 
 Thanks,
 
 Martin Gräßlin
 


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


Re: Review Request 118347: [klipper] Port from KIntSpinBox to QSpinBox

2014-05-29 Thread David Edmundson

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


Unfortunately this way we cannot have a plural aware suffix.

Then why port away?
Linking against kde4libsupport seems like the lesser of two evils. 

- David Edmundson


On May 27, 2014, 12:04 p.m., Martin Gräßlin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118347/
 ---
 
 (Updated May 27, 2014, 12:04 p.m.)
 
 
 Review request for Plasma.
 
 
 Repository: plasma-workspace
 
 
 Description
 ---
 
 [klipper] Port from KIntSpinBox to QSpinBox
 
 Unfortunately this way we cannot have a plural aware suffix.
 
 
 Diffs
 -
 
   klipper/configdialog.cpp 15901992db8d6fe28d800c4a18a3a6529cace1be 
   klipper/generalconfig.ui f0145752eb2dbd927392b682b6472695c793a588 
 
 Diff: https://git.reviewboard.kde.org/r/118347/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Martin Gräßlin
 


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


Re: Review Request 118347: [klipper] Port from KIntSpinBox to QSpinBox

2014-05-29 Thread Aleix Pol Gonzalez


 On May 29, 2014, 2:30 p.m., David Edmundson wrote:
  Unfortunately this way we cannot have a plural aware suffix.
  
  Then why port away?
  Linking against kde4libsupport seems like the lesser of two evils.

Well then we need to decide how we're going to get to the proper fix. We can't 
just decide it's good enough to depend on kdelibs4.


- Aleix


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


On May 27, 2014, 12:04 p.m., Martin Gräßlin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118347/
 ---
 
 (Updated May 27, 2014, 12:04 p.m.)
 
 
 Review request for Plasma.
 
 
 Repository: plasma-workspace
 
 
 Description
 ---
 
 [klipper] Port from KIntSpinBox to QSpinBox
 
 Unfortunately this way we cannot have a plural aware suffix.
 
 
 Diffs
 -
 
   klipper/configdialog.cpp 15901992db8d6fe28d800c4a18a3a6529cace1be 
   klipper/generalconfig.ui f0145752eb2dbd927392b682b6472695c793a588 
 
 Diff: https://git.reviewboard.kde.org/r/118347/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Martin Gräßlin
 


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


Re: Review Request 118347: [klipper] Port from KIntSpinBox to QSpinBox

2014-05-29 Thread David Edmundson


 On May 29, 2014, 2:30 p.m., David Edmundson wrote:
  Unfortunately this way we cannot have a plural aware suffix.
  
  Then why port away?
  Linking against kde4libsupport seems like the lesser of two evils.
 
 Aleix Pol Gonzalez wrote:
 Well then we need to decide how we're going to get to the proper fix. We 
 can't just decide it's good enough to depend on kdelibs4.

We also can't just break things.

Options are:
 1) restore KIntSpinBox to kwidgetaddons
 2) fix Qt, and drop the lib dependency once we have 5.4

I was just looking at Qt translator code. Detecting if something is singular or 
plural is not an easy task, and you need to have the locale tell you. There's 
not a nice system that fits in with Qt's API. I guess we can file a bug report 
anyway and see what happens.


- David


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


On May 27, 2014, 12:04 p.m., Martin Gräßlin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118347/
 ---
 
 (Updated May 27, 2014, 12:04 p.m.)
 
 
 Review request for Plasma.
 
 
 Repository: plasma-workspace
 
 
 Description
 ---
 
 [klipper] Port from KIntSpinBox to QSpinBox
 
 Unfortunately this way we cannot have a plural aware suffix.
 
 
 Diffs
 -
 
   klipper/configdialog.cpp 15901992db8d6fe28d800c4a18a3a6529cace1be 
   klipper/generalconfig.ui f0145752eb2dbd927392b682b6472695c793a588 
 
 Diff: https://git.reviewboard.kde.org/r/118347/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Martin Gräßlin
 


___
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.

2014-05-29 Thread David Edmundson

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


This doesn't look great to me.
We'd have to release another 4.x.

- David Edmundson


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 118290: Streamline Comment fields of KCMs

2014-05-29 Thread David Edmundson

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

Ship it!


- David Edmundson


On May 27, 2014, 12:03 a.m., Sebastian Kügler wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118290/
 ---
 
 (Updated May 27, 2014, 12:03 a.m.)
 
 
 Review request for Localization and Translation (l10n), Plasma and KDE 
 Usability.
 
 
 Repository: plasma-desktop
 
 
 Description
 ---
 
 Streamline Comment fields of KCMs
 
 This patch applies a common language and type-setting to the
 systemsettings modules in kde-workspace.
 
 Considerations:
 - The comment field might repeat the name, or give more detail about the
   specific settings on this page, this makes sense with how
   systemsettings and kcmshell present it
 - Mentioning the words settings, configure, options, etc. is avoided --
   it is clear from the context that these are settings and options.
 - Title-case throughout in line with human interface guidelines, see
   http://techbase.kde.org/Projects/Usability/HIG/Capitalization
 - The comment ends up being the title, so the
 - tech slang is avoided as much as possible, but left in where really
   necessary (hi Phonon), there were a few mentions of ~KDE Settings,
   which don't make sense in a properly branded universe.
 - I've left the Name field mostly untouched, as that one is key for
   the user to find the right module in systemsettings' icon view and in
   the sidebars
 
 The same treatment needs to be done for a bunch of other KCMs that we
 end up installing from other repos. This is a good start, however.
 
 
 Diffs
 -
 
   kcms/access/kcmaccess.desktop f859504 
   kcms/autostart/autostart.desktop 5a1048e 
   kcms/bell/bell.desktop 906aff7 
   kcms/colors/colors.desktop 409f665 
   kcms/componentchooser/componentchooser.desktop 214c4f1 
   kcms/dateandtime/clock.desktop acaca1a 
   kcms/desktoppaths/desktoppath.desktop da351d9 
   kcms/desktoptheme/desktoptheme.desktop 602f04c 
   kcms/emoticons/emoticons.desktop bbbd24a 
   kcms/fonts/fonts.desktop 99a88c4 
   kcms/formats/formats.desktop 8a95b61 
   kcms/hardware/display/display.desktop 0d8b7e6 
   kcms/hardware/joystick/joystick.desktop e5ff24a 
   kcms/icons/icons.desktop 1dfcf8c 
   kcms/input/cursortheme.desktop 4a5feed 
   kcms/input/mouse.desktop 5df856e 
   kcms/kded/kcmkded.desktop 76e299d 
   kcms/keyboard/kcm_keyboard.desktop 01f9542 
   kcms/keys/keys.desktop 30a4bc8 
   kcms/kfontinst/kcmfontinst/fontinst.desktop 6b13725 
   kcms/knotify/kcmnotify.desktop cd25cd4 
   kcms/ksmserver/kcmsmserver.desktop c461be7 
   kcms/ksplash/ksplashthememgr.desktop 711fbc3 
   kcms/launch/kcmlaunch.desktop fbb3ca6 
   kcms/phonon/kcm_phonon.desktop 46eecff 
   kcms/spellchecking/spellchecking.desktop 92a76cf 
   kcms/standard_actions/standard_actions.desktop 54a3ea8 
   kcms/style/style.desktop fd19bc4 
   kcms/workspaceoptions/workspaceoptions.desktop 3e31866 
 
 Diff: https://git.reviewboard.kde.org/r/118290/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Sebastian Kügler
 


___
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.

2014-05-29 Thread 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 118073: Use QHostInfo in kio_nfs

2014-05-29 Thread David Edmundson

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

Ship it!



nfs/kio_nfs.cpp
https://git.reviewboard.kde.org/r/118073/#comment40860

I don't think so.

Read qhostinfo_unix.cpp localDomainName

it has all sorts of fallbacks.


- David Edmundson


On May 10, 2014, 2:42 p.m., Alexander Richardson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118073/
 ---
 
 (Updated May 10, 2014, 2:42 p.m.)
 
 
 Review request for Plasma.
 
 
 Repository: kio-extras
 
 
 Description
 ---
 
 Use QHostInfo in kio_nfs
 
 
 Diffs
 -
 
   nfs/CMakeLists.txt dfc6eae3fad51eee0e736970da9520959eeed1f5 
   nfs/kio_nfs.cpp 293df2529a2488b7970dbe81a5ec202bc6a22742 
 
 Diff: https://git.reviewboard.kde.org/r/118073/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Alexander Richardson
 


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


Re: Review Request 118251: Small improvments to FadingNode and IconItem

2014-05-29 Thread David Edmundson


 On May 22, 2014, 10:02 a.m., David Edmundson wrote:
  src/declarativeimports/core/fadingnode.cpp, line 87
  https://git.reviewboard.kde.org/r/118251/diff/1/?file=274043#file274043line87
 
  Now it will animate backwards?
  
  Unless you change the vertex shader too.
 
 Martin Gräßlin wrote:
 this is really confusing. Why is there a check on target changed and then 
 source gets bound? I might have gotten that wrong but it looked strange. 
 Could you please elaborate on how that's supposed to work?
 
 David Edmundson wrote:
 Oooh! You're right. Clearly something is off.
 
 I think we might have to then swap round tex1 and tex2 in this line:
 gl_FragColor.a = mix(tex1.a, tex2.a, 
 
 before submitting could you try it on a super slow animation and check it 
 still animates correctly
 
 Martin Gräßlin wrote:
 no, I cannot animate at all, see 
 https://bugreports.qt-project.org/browse/QTBUG-39182

I tested this, you do need to swap round tex1 and text2 in my mix line in the 
shader.

You can either do that and ship it, or I can ship for yoU?


- David


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


On May 22, 2014, 6:02 a.m., Martin Gräßlin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118251/
 ---
 
 (Updated May 22, 2014, 6:02 a.m.)
 
 
 Review request for Plasma and David Edmundson.
 
 
 Repository: plasma-framework
 
 
 Description
 ---
 
 Small improvments to FadingNode and IconItem
 
 * trigger an update after the animation finished as the IconItem needs to
   switch to SVGTextureNode again
 * Don't connect needlessly to a lambda slot
 * FadingMaterialShader had the texture bind swapped
 * Fix virtual method hidden warning in FadingMaterialShader
 * Use dynamic_cast instead of static_cast to convert to SVGTextureNode
   or FadingNode.
 
 
 Diffs
 -
 
   src/declarativeimports/core/fadingnode.cpp 
 207eff3902848a3c76bf1ec62d73877d6d9d0df7 
   src/declarativeimports/core/iconitem.cpp 
 bdf5e286c9bff3ebd0b124f1593de63ed4232515 
 
 Diff: https://git.reviewboard.kde.org/r/118251/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Martin Gräßlin
 


___
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.

2014-05-29 Thread 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


Re: Review Request 118382: [klipper] Fix i18n

2014-05-29 Thread Lasse Liehu


 On May 29, 2014, 2:20 p.m., David Edmundson wrote:
  I'm not very happy giving a ship it to something that is ported but 
  completely untested. We end up with code we think works, but doesn't work 
  properly; which is worse than not touching it at all.
  
  It should be possible to download the x-test translations? Lets ask i18n if 
  we need to.

Tested that this fix works. Most strings are now translated. Some strings 
remain untranslated, but they seem to come from somewhere else.

x-test from /trunk/l10n-kf5 works the same way it does in other l10n branches. 
As environment variable LANGUAGE=x-test is used instead of KDE_LANG=x-test.


- Lasse


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


On May 28, 2014, 3:22 p.m., Martin Gräßlin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118382/
 ---
 
 (Updated May 28, 2014, 3:22 p.m.)
 
 
 Review request for Plasma.
 
 
 Repository: plasma-workspace
 
 
 Description
 ---
 
 [klipper] Fix i18n
 
 
 Diffs
 -
 
   klipper/CMakeLists.txt 7a57b30b8b59a77c3d0702095dd72bb275d22e57 
   klipper/main.cpp 153a03160717431eaaa95e77903ab43de9b07fcb 
 
 Diff: https://git.reviewboard.kde.org/r/118382/diff/
 
 
 Testing
 ---
 
 I still don't know how to test i18n with kf5 :-)
 
 
 Thanks,
 
 Martin Gräßlin
 


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


Re: Review Request 118347: [klipper] Port from KIntSpinBox to QSpinBox

2014-05-29 Thread David Edmundson


 On May 29, 2014, 2:30 p.m., David Edmundson wrote:
  Unfortunately this way we cannot have a plural aware suffix.
  
  Then why port away?
  Linking against kde4libsupport seems like the lesser of two evils.
 
 Aleix Pol Gonzalez wrote:
 Well then we need to decide how we're going to get to the proper fix. We 
 can't just decide it's good enough to depend on kdelibs4.
 
 David Edmundson wrote:
 We also can't just break things.
 
 Options are:
  1) restore KIntSpinBox to kwidgetaddons
  2) fix Qt, and drop the lib dependency once we have 5.4
 
 I was just looking at Qt translator code. Detecting if something is 
 singular or plural is not an easy task, and you need to have the locale tell 
 you. There's not a nice system that fits in with Qt's API. I guess we can 
 file a bug report anyway and see what happens.
 


Qt bug opened https://bugreports.qt-project.org/browse/QTBUG-39356


- David


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


On May 27, 2014, 12:04 p.m., Martin Gräßlin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118347/
 ---
 
 (Updated May 27, 2014, 12:04 p.m.)
 
 
 Review request for Plasma.
 
 
 Repository: plasma-workspace
 
 
 Description
 ---
 
 [klipper] Port from KIntSpinBox to QSpinBox
 
 Unfortunately this way we cannot have a plural aware suffix.
 
 
 Diffs
 -
 
   klipper/configdialog.cpp 15901992db8d6fe28d800c4a18a3a6529cace1be 
   klipper/generalconfig.ui f0145752eb2dbd927392b682b6472695c793a588 
 
 Diff: https://git.reviewboard.kde.org/r/118347/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Martin Gräßlin
 


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


Re: Review Request 118390: Powerdevil KCM Updates

2014-05-29 Thread Andrew Lake


 On May 29, 2014, 8:13 a.m., Thomas Pfeiffer wrote:
  Good improvements in general!
  There are two changes which are not HIG-compliant:
  - Control labels which are written next to the controls (checkboxes, spin 
  boxes etc.) do not use Title Capitalization. See 
  http://techbase.kde.org/Projects/Usability/HIG/Capitalization for details. 
  Therefore most of the capitalization changes have to be reverted, I'm 
  afraid.
  - For grouping controls (as in the case of Battery Levels and Events) the 
  HIG recommends using group boxes (see 
  http://techbase.kde.org/Projects/Usability/HIG/GroupBox ) instead of simply 
  spacing
  Apart from those two things, the changes look fine to me!

 - For grouping controls (as in the case of Battery Levels and Events) the HIG 
 recommends using group boxes (see 
 http://techbase.kde.org/Projects/Usability/HIG/GroupBox ) instead of simply 
 spacing

Hmm, may be time to update that portion of the HIG since, from a visual design 
standpoint, spacing can be quite effective at grouping visual elements while 
avoiding more visual chrome. In this case, for the Advanced page, a bit more 
spacing between the Battery Levels group and the Events group should provide 
enough visual distinction between the groups. Also, the checkboxes in the 
Events group can probably be left-aligned under Events since they're unrelated 
to the widgets from the Battery Levels group and likely better match the 
alignment guidelines for making the checkboxes span two columns 
(http://techbase.kde.org/Projects/Usability/HIG/Alignment).

Otherwise, the changes look quite nice to me!


- Andrew


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


On May 28, 2014, 11:33 p.m., Sebastian Kügler wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118390/
 ---
 
 (Updated May 28, 2014, 11:33 p.m.)
 
 
 Review request for Plasma, Solid and KDE Usability.
 
 
 Repository: powerdevil
 
 
 Description
 ---
 
 This is a series of UI updates I've applied to the powerdevil KCMs. You can 
 find them as individual patch series in the sebas/kcmupdates branch.
 
 General:
 - Less icon usage, especially in the form layouts
 - Title casing throughout
 - Better HIG compliance (not 100%, but improved)
 - Energy Saving / Actions UI now scales with dialog
 - Proper usage of FormLayouts
 - A bunch of cleanups of dead code
 - Parenting fixes
 
 In Detail:
 
 * Improve Advanced Settings page
 
 - Use a QFormLayout, and do it properly
 - Fix up spacing and alignment
 - Remove icons before titles
 - Use Title Case for Labels
 - Shorter labels for better readability
 
 * Clean up brightness-OSD-related dead code
 
 
 * Make powerdevil actions layout stretch out horizontally
 
 
 * Compile-time connections in actionconfigwidget
 
 
 * widget and layout parenting fixes
 
 
 * No bold font, increased spacing instead
 
 This removes the bold fonts from the checkboxes, as that is non-standard
 in the HIG. In order to make it look a bit more structured between the
 sections, a bit of spacing is added.
 
 * Remove icons from actions
 
 The icons are really small and add more visual noise than being useful.
 
 * Streamline Comments of KCMs
 
 
 * Clean up dead code
 
 
 Diffs
 -
 
   kcmodule/profiles/profileEditPage.ui 
 dc2657943a2bb63f137ad11b77b254b3782f0407 
   kcmodule/profiles/powerdevilprofilesconfig.desktop 
 e95b908dfea30a4a15ccd8c89d4ceb36256e2185 
   kcmodule/profiles/EditPage.cpp a674ccbdd76ba682c7b7111f2f9d0b9123ffe8b8 
   kcmodule/global/powerdevilglobalconfig.desktop 
 1d4aa2238b2fa02dfa984f9163f133c941ff509e 
   kcmodule/global/generalPage.ui 2ce7cef98e30f872ea9233edc7882deea134fdd3 
   kcmodule/global/GeneralPage.cpp d025e429df7ca191bf56443b2bd30353a74993a9 
   kcmodule/common/actioneditwidget.cpp 
 4c67b4feeb2cff0df2e397f44ffba2926bbb7276 
   kcmodule/common/actionconfigwidget.cpp 
 2161c84b68e5fc3b56240a482ed77825ce4a5f03 
   kcmodule/activities/powerdevilactivitiesconfig.desktop 
 9ee81b55ec7f4685adb5d102cb21df81f17cf3e3 
   kcmodule/activities/activitypage.cpp 
 48e9c6c75f2ab95ce52c96b586475dbb4d6022fd 
   daemon/actions/dpms/powerdevildpmsactionconfig.cpp 
 4d96273697f11864716dae0204009d52264be99f 
   daemon/actions/bundled/runscriptconfig.cpp 
 c0d3adb6651320702bb6503e505b6ebdc06d6004 
   daemon/actions/bundled/keyboardbrightnesscontrolconfig.cpp 
 31772679560a256dcf9f66aefbb4cf02351bd5d3 
   daemon/actions/bundled/keyboardbrightnesscontrol.cpp 
 44dbcd87c06f5dce6d1ff10697837a60cdcc5f29 
   daemon/actions/bundled/dimdisplayconfig.cpp 
 f683935994e4a5a08fd7da6ceb95ba7b726ac708 
   daemon/actions/bundled/brightnesscontrolconfig.cpp 
 

Re: Review Request 118388: rename systemsettings binary to systemsettings5

2014-05-29 Thread Hrvoje Senjan


 On May 29, 2014, 9:10 a.m., Ben Cooksley wrote:
  Code wise, this change looks fine. In terms of renaming the desktop files - 
  i'm fine with changing the filenames, but please don't change the name of 
  the application itself. Ideally the KF5 based system settings will still be 
  able to set configuration details relevant for KDE 4 applications.

Ideally the KF5 based system settings will still be able to set configuration 
details relevant for KDE 4 applications.

that sounds great to me - just that it would need work in every kcm module. 
additional caveats:
what if there's only 4.x variant of the kcm?
an option is removed in KF5 variant?
most importantly - how to implement it (different config locations, etc)? =) 


- Hrvoje


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


On May 28, 2014, 9:32 p.m., Hrvoje Senjan wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118388/
 ---
 
 (Updated May 28, 2014, 9:32 p.m.)
 
 
 Review request for Plasma and Ben Cooksley.
 
 
 Repository: systemsettings
 
 
 Description
 ---
 
 while workspace might not be targeted to co-exist with 4.x variant - 
 systemsettings should IMHO be able to co-exist. not only workspace components 
 are adjusting in there, and telling people to do kcmshell$notinstalledvariant 
 $wantedkcm is very user-unfriendly...
 one TODO if this gets a green light, is to rename desktop files, so people 
 know which variant they are opening.
 
 
 Diffs
 -
 
   app/systemsettings.desktop 5f27318 
   app/kdesystemsettings.desktop 946d498 
   app/CMakeLists.txt c45f7e7 
 
 Diff: https://git.reviewboard.kde.org/r/118388/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Hrvoje Senjan
 


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


Review Request 118406: Notify the user if the location containing the media is inaccessible.

2014-05-29 Thread R.Harish Navnit

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

Review request for Plasma, Shantanu Tushar and Sinny Kumari.


Bugs: 333764
http://bugs.kde.org/show_bug.cgi?id=333764


Repository: plasma-mediacenter


Description
---

If a media(in a playlist) is located in an inaccessible location, then the user 
is notified about the same. 


Diffs
-

  mediaelements/mediaplayer/MediaPlayer.qml 98f1d2c 

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


Testing
---

1. Load media to a playlist.
2. Unmount the device containing media.
3. Check if the user is notified of the location being inaccessible
   --yes, the user is notified
4. Mount the device containing media and play a media from playlist.
   -- The media plays properly.


Thanks,

R.Harish  Navnit

___
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.

2014-05-29 Thread Matthew Dawson


 On May 29, 2014, 10:57 a.m., David Edmundson wrote:
  This doesn't look great to me.
  We'd have to release another 4.x.

Is this too big for the KDE 4.13.x releases?  It doesn't change the default 
behaviour, and as discussed in: https://git.reviewboard.kde.org/r/115602/ , 
this is the only way to have a kactivities installed from both the KDE4 
releases and frameworks.

If it isn't wanted, I'll push it into my distribution (Gentoo) as a downstream 
patch.  But, I have a feeling most distributions will do this, wasting effort.


- Matthew


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


On May 27, 2014, 1: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, 1: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.

2014-05-29 Thread Matthew Dawson


 On May 29, 2014, 11:02 a.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.
 
 Alex Merry wrote:
 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.

Sorry for the confusion, I wasn't sure where exactly to address the RR, so I 
addressed it to all the places that seemed relevant.

If David is ok with this patch, and it has a chance of going in I'll change it 
to use an explicit option.  I originally took the path of least resistance, as 
most users wouldn't see this or care.  But the explicit option would make the 
option definitely easier to understand.


- Matthew


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


On May 27, 2014, 1: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, 1: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 118406: Notify the user if the location containing the media is inaccessible.

2014-05-29 Thread Bhushan Shah

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



mediaelements/mediaplayer/MediaPlayer.qml
https://git.reviewboard.kde.org/r/118406/#comment40863

use i18n here so text can be translated.

i18n(Your text)


- Bhushan Shah


On May 29, 2014, 10:10 p.m., R.Harish  Navnit wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118406/
 ---
 
 (Updated May 29, 2014, 10:10 p.m.)
 
 
 Review request for Plasma, Shantanu Tushar and Sinny Kumari.
 
 
 Bugs: 333764
 http://bugs.kde.org/show_bug.cgi?id=333764
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 If a media(in a playlist) is located in an inaccessible location, then the 
 user is notified about the same. 
 
 
 Diffs
 -
 
   mediaelements/mediaplayer/MediaPlayer.qml 98f1d2c 
 
 Diff: https://git.reviewboard.kde.org/r/118406/diff/
 
 
 Testing
 ---
 
 1. Load media to a playlist.
 2. Unmount the device containing media.
 3. Check if the user is notified of the location being inaccessible
--yes, the user is notified
 4. Mount the device containing media and play a media from playlist.
-- The media plays properly.
 
 
 Thanks,
 
 R.Harish  Navnit
 


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


Re: Review Request 118357: Disable the agenda part of the calendar

2014-05-29 Thread Martin Klapetek

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


Ping?

- Martin Klapetek


On May 27, 2014, 6 p.m., Martin Klapetek wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118357/
 ---
 
 (Updated May 27, 2014, 6 p.m.)
 
 
 Review request for Plasma.
 
 
 Repository: plasma-workspace
 
 
 Description
 ---
 
 As agreed on irc (someday ago), the agenda part is useless until there's a 
 proper agenda backend and should just be hidden. Here's a patch simply hiding 
 the left part - it's easier to do just visible: false than comment it out 
 and then comment out/remove all the lines referencing parts of the agenda, 
 also cleaner.
 
 I added a big comment at the file beginning, I'll fill the commit sha after 
 committing so it can be easily reverted (the comment will be separate commit).
 
 Screenshot attached.
 
 Oh and you might want to remove the clock from panel and readd it/remove 
 plasma config as the popup size seems to be saved and so after this patch you 
 may still get the original-sized half-empty dialog.
 
 
 Diffs
 -
 
   applets/digital-clock/package/contents/ui/CalendarView.qml 43812a4 
 
 Diff: https://git.reviewboard.kde.org/r/118357/diff/
 
 
 Testing
 ---
 
 
 File Attachments
 
 
 Calendar without agenda
   
 https://git.reviewboard.kde.org/media/uploaded/files/2014/05/27/84880c21-1367-4d48-88db-7747553c40f5__plasma_cal1.png
 
 
 Thanks,
 
 Martin Klapetek
 


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


Re: Review Request 118290: Streamline Comment fields of KCMs

2014-05-29 Thread Commit Hook

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


This review has been submitted with commit 
7128a3aca1cbc1397b0e648d78cd711e94afe9b9 by Sebastian Kügler to branch master.

- Commit Hook


On May 27, 2014, 12:03 a.m., Sebastian Kügler wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118290/
 ---
 
 (Updated May 27, 2014, 12:03 a.m.)
 
 
 Review request for Localization and Translation (l10n), Plasma and KDE 
 Usability.
 
 
 Repository: plasma-desktop
 
 
 Description
 ---
 
 Streamline Comment fields of KCMs
 
 This patch applies a common language and type-setting to the
 systemsettings modules in kde-workspace.
 
 Considerations:
 - The comment field might repeat the name, or give more detail about the
   specific settings on this page, this makes sense with how
   systemsettings and kcmshell present it
 - Mentioning the words settings, configure, options, etc. is avoided --
   it is clear from the context that these are settings and options.
 - Title-case throughout in line with human interface guidelines, see
   http://techbase.kde.org/Projects/Usability/HIG/Capitalization
 - The comment ends up being the title, so the
 - tech slang is avoided as much as possible, but left in where really
   necessary (hi Phonon), there were a few mentions of ~KDE Settings,
   which don't make sense in a properly branded universe.
 - I've left the Name field mostly untouched, as that one is key for
   the user to find the right module in systemsettings' icon view and in
   the sidebars
 
 The same treatment needs to be done for a bunch of other KCMs that we
 end up installing from other repos. This is a good start, however.
 
 
 Diffs
 -
 
   kcms/access/kcmaccess.desktop f859504 
   kcms/autostart/autostart.desktop 5a1048e 
   kcms/bell/bell.desktop 906aff7 
   kcms/colors/colors.desktop 409f665 
   kcms/componentchooser/componentchooser.desktop 214c4f1 
   kcms/dateandtime/clock.desktop acaca1a 
   kcms/desktoppaths/desktoppath.desktop da351d9 
   kcms/desktoptheme/desktoptheme.desktop 602f04c 
   kcms/emoticons/emoticons.desktop bbbd24a 
   kcms/fonts/fonts.desktop 99a88c4 
   kcms/formats/formats.desktop 8a95b61 
   kcms/hardware/display/display.desktop 0d8b7e6 
   kcms/hardware/joystick/joystick.desktop e5ff24a 
   kcms/icons/icons.desktop 1dfcf8c 
   kcms/input/cursortheme.desktop 4a5feed 
   kcms/input/mouse.desktop 5df856e 
   kcms/kded/kcmkded.desktop 76e299d 
   kcms/keyboard/kcm_keyboard.desktop 01f9542 
   kcms/keys/keys.desktop 30a4bc8 
   kcms/kfontinst/kcmfontinst/fontinst.desktop 6b13725 
   kcms/knotify/kcmnotify.desktop cd25cd4 
   kcms/ksmserver/kcmsmserver.desktop c461be7 
   kcms/ksplash/ksplashthememgr.desktop 711fbc3 
   kcms/launch/kcmlaunch.desktop fbb3ca6 
   kcms/phonon/kcm_phonon.desktop 46eecff 
   kcms/spellchecking/spellchecking.desktop 92a76cf 
   kcms/standard_actions/standard_actions.desktop 54a3ea8 
   kcms/style/style.desktop fd19bc4 
   kcms/workspaceoptions/workspaceoptions.desktop 3e31866 
 
 Diff: https://git.reviewboard.kde.org/r/118290/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Sebastian Kügler
 


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


Re: Review Request 118290: Streamline Comment fields of KCMs

2014-05-29 Thread Sebastian Kügler

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

(Updated May 29, 2014, 11:28 p.m.)


Status
--

This change has been marked as submitted.


Review request for Localization and Translation (l10n), Plasma and KDE 
Usability.


Repository: plasma-desktop


Description
---

Streamline Comment fields of KCMs

This patch applies a common language and type-setting to the
systemsettings modules in kde-workspace.

Considerations:
- The comment field might repeat the name, or give more detail about the
  specific settings on this page, this makes sense with how
  systemsettings and kcmshell present it
- Mentioning the words settings, configure, options, etc. is avoided --
  it is clear from the context that these are settings and options.
- Title-case throughout in line with human interface guidelines, see
  http://techbase.kde.org/Projects/Usability/HIG/Capitalization
- The comment ends up being the title, so the
- tech slang is avoided as much as possible, but left in where really
  necessary (hi Phonon), there were a few mentions of ~KDE Settings,
  which don't make sense in a properly branded universe.
- I've left the Name field mostly untouched, as that one is key for
  the user to find the right module in systemsettings' icon view and in
  the sidebars

The same treatment needs to be done for a bunch of other KCMs that we
end up installing from other repos. This is a good start, however.


Diffs
-

  kcms/access/kcmaccess.desktop f859504 
  kcms/autostart/autostart.desktop 5a1048e 
  kcms/bell/bell.desktop 906aff7 
  kcms/colors/colors.desktop 409f665 
  kcms/componentchooser/componentchooser.desktop 214c4f1 
  kcms/dateandtime/clock.desktop acaca1a 
  kcms/desktoppaths/desktoppath.desktop da351d9 
  kcms/desktoptheme/desktoptheme.desktop 602f04c 
  kcms/emoticons/emoticons.desktop bbbd24a 
  kcms/fonts/fonts.desktop 99a88c4 
  kcms/formats/formats.desktop 8a95b61 
  kcms/hardware/display/display.desktop 0d8b7e6 
  kcms/hardware/joystick/joystick.desktop e5ff24a 
  kcms/icons/icons.desktop 1dfcf8c 
  kcms/input/cursortheme.desktop 4a5feed 
  kcms/input/mouse.desktop 5df856e 
  kcms/kded/kcmkded.desktop 76e299d 
  kcms/keyboard/kcm_keyboard.desktop 01f9542 
  kcms/keys/keys.desktop 30a4bc8 
  kcms/kfontinst/kcmfontinst/fontinst.desktop 6b13725 
  kcms/knotify/kcmnotify.desktop cd25cd4 
  kcms/ksmserver/kcmsmserver.desktop c461be7 
  kcms/ksplash/ksplashthememgr.desktop 711fbc3 
  kcms/launch/kcmlaunch.desktop fbb3ca6 
  kcms/phonon/kcm_phonon.desktop 46eecff 
  kcms/spellchecking/spellchecking.desktop 92a76cf 
  kcms/standard_actions/standard_actions.desktop 54a3ea8 
  kcms/style/style.desktop fd19bc4 
  kcms/workspaceoptions/workspaceoptions.desktop 3e31866 

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


Testing
---


Thanks,

Sebastian Kügler

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


Review Request 118413: Put the settings views in a ScrollView

2014-05-29 Thread Aleix Pol Gonzalez

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

Review request for Plasma.


Repository: plasma-desktop


Description
---

So far, if the page didn't fit the view, it wouldn't react or anything and it 
would just cut the contents out. With this change it will show scrollbars so it 
can be reached, if it's needed.


Diffs
-

  desktoppackage/contents/configuration/AppletConfiguration.qml ea6010a 

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


Testing
---


Thanks,

Aleix Pol Gonzalez

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


Re: Review Request 118382: [klipper] Fix i18n

2014-05-29 Thread Martin Gräßlin


 On May 29, 2014, 4:20 p.m., David Edmundson wrote:
  I'm not very happy giving a ship it to something that is ported but 
  completely untested. We end up with code we think works, but doesn't work 
  properly; which is worse than not touching it at all.
  
  It should be possible to download the x-test translations? Lets ask i18n if 
  we need to.
 
 Lasse Liehu wrote:
 Tested that this fix works. Most strings are now translated. Some strings 
 remain untranslated, but they seem to come from somewhere else.
 
 x-test from /trunk/l10n-kf5 works the same way it does in other l10n 
 branches. As environment variable LANGUAGE=x-test is used instead of 
 KDE_LANG=x-test.

@Lasse: thanks for testing.


- Martin


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


On May 28, 2014, 5:22 p.m., Martin Gräßlin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118382/
 ---
 
 (Updated May 28, 2014, 5:22 p.m.)
 
 
 Review request for Plasma.
 
 
 Repository: plasma-workspace
 
 
 Description
 ---
 
 [klipper] Fix i18n
 
 
 Diffs
 -
 
   klipper/CMakeLists.txt 7a57b30b8b59a77c3d0702095dd72bb275d22e57 
   klipper/main.cpp 153a03160717431eaaa95e77903ab43de9b07fcb 
 
 Diff: https://git.reviewboard.kde.org/r/118382/diff/
 
 
 Testing
 ---
 
 I still don't know how to test i18n with kf5 :-)
 
 
 Thanks,
 
 Martin Gräßlin
 


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


Re: Review Request 118381: [klipper] Reintroduce the dependency on prison

2014-05-29 Thread Commit Hook

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


This review has been submitted with commit 
f09e5a67de9b1e5a702735c8c5dc7cc7ed89b6d3 by Martin Gräßlin to branch master.

- Commit Hook


On May 28, 2014, 2:49 p.m., Martin Gräßlin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118381/
 ---
 
 (Updated May 28, 2014, 2:49 p.m.)
 
 
 Review request for Plasma and Johannes Huber.
 
 
 Repository: plasma-workspace
 
 
 Description
 ---
 
 [klipper] Port Barcode dialog from KDialog to QDialog
 
 
 [klipper] Reintroduce the dependency on prison
 
 
 Diffs
 -
 
   klipper/CMakeLists.txt 7a57b30b8b59a77c3d0702095dd72bb275d22e57 
   klipper/klipper.cpp 1ce578afb078b3b4e791e780975802082559bdf4 
 
 Diff: https://git.reviewboard.kde.org/r/118381/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Martin Gräßlin
 


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


Re: Review Request 118382: [klipper] Fix i18n

2014-05-29 Thread Martin Gräßlin

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

(Updated May 30, 2014, 5:54 a.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma.


Repository: plasma-workspace


Description
---

[klipper] Fix i18n


Diffs
-

  klipper/CMakeLists.txt 7a57b30b8b59a77c3d0702095dd72bb275d22e57 
  klipper/main.cpp 153a03160717431eaaa95e77903ab43de9b07fcb 

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


Testing
---

I still don't know how to test i18n with kf5 :-)


Thanks,

Martin Gräßlin

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


Re: Review Request 118381: [klipper] Reintroduce the dependency on prison

2014-05-29 Thread Martin Gräßlin

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

(Updated May 30, 2014, 5:54 a.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma and Johannes Huber.


Repository: plasma-workspace


Description
---

[klipper] Port Barcode dialog from KDialog to QDialog


[klipper] Reintroduce the dependency on prison


Diffs
-

  klipper/CMakeLists.txt 7a57b30b8b59a77c3d0702095dd72bb275d22e57 
  klipper/klipper.cpp 1ce578afb078b3b4e791e780975802082559bdf4 

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


Testing
---


Thanks,

Martin Gräßlin

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