Re: Review Request 114298: prevent ksplashqml crash when a theme does not exist or cannot be loaded

2013-12-04 Thread Pino Toscano

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



ksplash/ksplashqml/SplashApp.cpp


What about just calling
  ::exit(2)
to reach the stdlib function, avoiding the exitNow "wrapper"?


- Pino Toscano


On Dec. 4, 2013, 1:54 p.m., Harald Sitter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/114298/
> ---
> 
> (Updated Dec. 4, 2013, 1:54 p.m.)
> 
> 
> Review request for kde-workspace and Ivan Čukić.
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> ---
> 
> Do not crash when failing to load a theme, but instead exit.
> 
> When failing to load the theme's qml file QDeclarativeView goes into error
> state, at which point we want to exit to prevent nullptr access problems.
> 
> 
> Diffs
> -
> 
>   ksplash/ksplashqml/SplashApp.cpp 7c528d056ee680f69a6d3d60d1dbeeae9d548846 
> 
> Diff: http://git.reviewboard.kde.org/r/114298/diff/
> 
> 
> Testing
> ---
> 
> ksplashqml Foo --test
> 
> -> sigsev without patch
> -> exits with patch
> 
> 
> Thanks,
> 
> Harald Sitter
> 
>



Re: Review Request 112463: Port SMB kioslave to KF5/Qt5

2013-12-04 Thread Kevin Ottens

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



kioslave/smb/kio_smb.cpp


Can be removed



kioslave/smb/kio_smb_internal.h


Spurious space



kioslave/smb/kio_smb_internal.cpp


Could be the mode compact toString(QUrl::PrettyDecoded).toUtf8()

Avoids a temporary in the process.


- Kevin Ottens


On Dec. 1, 2013, 9:32 p.m., Mark Gaiser wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112463/
> ---
> 
> (Updated Dec. 1, 2013, 9:32 p.m.)
> 
> 
> Review request for KDE Runtime and KDE Frameworks.
> 
> 
> Repository: kde-runtime
> 
> 
> Description
> ---
> 
> This is the initial port! I added two TODO lines in the diff for parts where 
> i'm not sure if I've ported them correctly.
> Also, i needed a change in FindSamba.cmake to even get the samba detection 
> working. That reviewrequest is waiting here: 
> https://git.reviewboard.kde.org/r/112448/ you're probably OK if you still use 
> samba 3.x
> 
> Once i know that this is actually working then i will comment some qDebug 
> lines.
> 
> 
> Diffs
> -
> 
>   kioslave/CMakeLists.txt fc594e4 
>   kioslave/smb/CMakeLists.txt a3a2265 
>   kioslave/smb/kio_smb.h c2229ab 
>   kioslave/smb/kio_smb.cpp 2c2523a 
>   kioslave/smb/kio_smb_auth.cpp 4d236b4 
>   kioslave/smb/kio_smb_browse.cpp 5253be9 
>   kioslave/smb/kio_smb_dir.cpp ba80c86 
>   kioslave/smb/kio_smb_file.cpp 206526a 
>   kioslave/smb/kio_smb_internal.h 4b946c1 
>   kioslave/smb/kio_smb_internal.cpp e943844 
>   kioslave/smb/kio_smb_mount.cpp a5a7e8e 
>   kioslave/smb/kio_smb_win.h f1dcb6f 
>   kioslave/smb/kio_smb_win.cpp 14dd797 
> 
> Diff: http://git.reviewboard.kde.org/r/112463/diff/
> 
> 
> Testing
> ---
> 
> It compiles and gets loaded just fine. I tried testing this on an actual 
> samba share, but i kept getting a 111 error (connection refused) from kio_smb 
> so i'm hoping that is a local issue here. If someone else could try this out 
> and verify that it's either working or broken.
> 
> 
> Thanks,
> 
> Mark Gaiser
> 
>



Re: Review Request 112880: Added KColorSchemeToken class.

2013-12-04 Thread Denis Kuplyakov

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

(Updated Dec. 4, 2013, 7:32 p.m.)


Status
--

This change has been discarded.


Review request for KDE Frameworks and kdelibs.


Repository: kdelibs


Description
---

It is wrapper to access KColorScheme's methods from QML code.
Also added Q_GADGET to KColorScheme to enable Q_ENUMS using, to make them 
accessible from QML code.

As it will be accepted, QML-clone of KgPopupItem will be posted for review to 
libkdegames, as it uses it to access KDE's color theme.

More info:
* search for "KDE theme colors API for QML" thread at kdelibs and kdegames 
mailinglists *


Diffs
-

  kdeui/CMakeLists.txt b439e04 
  includes/CMakeLists.txt cdf0143 
  includes/KColorSchemeToken PRE-CREATION 
  kdeui/colors/kcolorscheme.h 17570fd 
  kdeui/colors/kcolorscheme.cpp a6650ac 
  kdeui/colors/kcolorschemetoken.h PRE-CREATION 
  kdeui/colors/kcolorschemetoken.cpp PRE-CREATION 

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


Testing
---

I've tested it with KReversi's deniskup/gsoc2013/newdesign branch.


Thanks,

Denis Kuplyakov



Re: Review Request 112463: Port SMB kioslave to KF5/Qt5

2013-12-04 Thread Mark Gaiser
Bump.

Can anyone look at this?


On Sun, Dec 1, 2013 at 10:32 PM, Mark Gaiser  wrote:

>This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112463/
>   Review request for KDE Runtime and KDE Frameworks.
> By Mark Gaiser.
>
> *Updated Dec. 1, 2013, 9:32 p.m.*
> Changes
>
> Updated diff since Dawit is done with his patches.
> I tried to be as thorough as possible with this porting. You won't see any 
> new notices or warning coming from this patch. I at least didn't saw any in 
> my test compiles.
>
> As for testing this. I did manage to test it and got file listings back. So 
> that makes me think that it's ported properly and working. But i have to say 
> that testing this is very tricky! The applications that one would normally 
> use to verify if everything is OK (aka, dolphin) is not possible because 
> dolphin isn't ported. Testing it in my new app "Accretion" is possible, but 
> not reliable since that is in heavy development. If something doesn't show up 
> there it doesn't mean the slave is broken :)
>
> So this patch is partly based on guess work. It looks like it's working fine.
> On the other hand, if it isn't i'm likely the one finding it out anyway since 
> i'm digging around a lot in this area lately.
>
>   *Repository: * kde-runtime
> Description
>
> This is the initial port! I added two TODO lines in the diff for parts where 
> i'm not sure if I've ported them correctly.
> Also, i needed a change in FindSamba.cmake to even get the samba detection 
> working. That reviewrequest is waiting here: 
> https://git.reviewboard.kde.org/r/112448/ you're probably OK if you still use 
> samba 3.x
>
> Once i know that this is actually working then i will comment some qDebug 
> lines.
>
>   Testing
>
> It compiles and gets loaded just fine. I tried testing this on an actual 
> samba share, but i kept getting a 111 error (connection refused) from kio_smb 
> so i'm hoping that is a local issue here. If someone else could try this out 
> and verify that it's either working or broken.
>
>   Diffs (updated)
>
>- kioslave/CMakeLists.txt (fc594e4)
>- kioslave/smb/CMakeLists.txt (a3a2265)
>- kioslave/smb/kio_smb.h (c2229ab)
>- kioslave/smb/kio_smb.cpp (2c2523a)
>- kioslave/smb/kio_smb_auth.cpp (4d236b4)
>- kioslave/smb/kio_smb_browse.cpp (5253be9)
>- kioslave/smb/kio_smb_dir.cpp (ba80c86)
>- kioslave/smb/kio_smb_file.cpp (206526a)
>- kioslave/smb/kio_smb_internal.h (4b946c1)
>- kioslave/smb/kio_smb_internal.cpp (e943844)
>- kioslave/smb/kio_smb_mount.cpp (a5a7e8e)
>- kioslave/smb/kio_smb_win.h (f1dcb6f)
>- kioslave/smb/kio_smb_win.cpp (14dd797)
>
> View Diff 
>


Re: Review Request 114298: prevent ksplashqml crash when a theme does not exist or cannot be loaded

2013-12-04 Thread Martin Klapetek

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

Ship it!



ksplash/ksplashqml/SplashApp.cpp


add {} 


- Martin Klapetek


On Dec. 4, 2013, 1:54 p.m., Harald Sitter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/114298/
> ---
> 
> (Updated Dec. 4, 2013, 1:54 p.m.)
> 
> 
> Review request for kde-workspace and Ivan Čukić.
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> ---
> 
> Do not crash when failing to load a theme, but instead exit.
> 
> When failing to load the theme's qml file QDeclarativeView goes into error
> state, at which point we want to exit to prevent nullptr access problems.
> 
> 
> Diffs
> -
> 
>   ksplash/ksplashqml/SplashApp.cpp 7c528d056ee680f69a6d3d60d1dbeeae9d548846 
> 
> Diff: http://git.reviewboard.kde.org/r/114298/diff/
> 
> 
> Testing
> ---
> 
> ksplashqml Foo --test
> 
> -> sigsev without patch
> -> exits with patch
> 
> 
> Thanks,
> 
> Harald Sitter
> 
>



Review Request 114298: prevent ksplashqml crash when a theme does not exist or cannot be loaded

2013-12-04 Thread Harald Sitter

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

Review request for kde-workspace and Ivan Čukić.


Repository: kde-workspace


Description
---

Do not crash when failing to load a theme, but instead exit.

When failing to load the theme's qml file QDeclarativeView goes into error
state, at which point we want to exit to prevent nullptr access problems.


Diffs
-

  ksplash/ksplashqml/SplashApp.cpp 7c528d056ee680f69a6d3d60d1dbeeae9d548846 

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


Testing
---

ksplashqml Foo --test

-> sigsev without patch
-> exits with patch


Thanks,

Harald Sitter



Re: Review Request 114201: define property X-KDE-PluginKeyword in kdelibs/kio/kcmodule.desktop

2013-12-04 Thread Burkhard Lück

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

(Updated Dec. 4, 2013, 11:43 a.m.)


Status
--

This change has been marked as submitted.


Review request for kdelibs and David Faure.


Repository: kdelibs


Description
---

This is required to make https://git.reviewboard.kde.org/r/111851/ work properly

I don't know what to document here, the only hint I found in:
http://websvn.kde.org/?view=revision&revision=705672
Log Message: port to KPluginFactory


Diffs
-

  kio/kcmodule.desktop 2a978a5 

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


Testing
---


Thanks,

Burkhard Lück



Re: Review Request 114201: define property X-KDE-PluginKeyword in kdelibs/kio/kcmodule.desktop

2013-12-04 Thread Commit Hook

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


This review has been submitted with commit 
21ad59cc03e9af11b784b5ab9de56f287a11b106 by Burkhard Lück to branch master.

- Commit Hook


On Nov. 29, 2013, 12:35 p.m., Burkhard Lück wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/114201/
> ---
> 
> (Updated Nov. 29, 2013, 12:35 p.m.)
> 
> 
> Review request for kdelibs and David Faure.
> 
> 
> Repository: kdelibs
> 
> 
> Description
> ---
> 
> This is required to make https://git.reviewboard.kde.org/r/111851/ work 
> properly
> 
> I don't know what to document here, the only hint I found in:
> http://websvn.kde.org/?view=revision&revision=705672
> Log Message: port to KPluginFactory
> 
> 
> Diffs
> -
> 
>   kio/kcmodule.desktop 2a978a5 
> 
> Diff: http://git.reviewboard.kde.org/r/114201/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Burkhard Lück
> 
>