Re: Refocusing the tech preview

2013-12-28 Thread David Faure
On Saturday 21 December 2013 12:42:04 Kevin Ottens wrote:
 Hello people,
 
 I see that new tasks appeared in the wiki, that's great, they were clearly
 missing. That said, they were all earmarked for the tech preview, so I took
 the liberty to move most of them to the list about the final release. We
 should get the tech preview out of the door ASAP now. It won't be perfect,
 that's fine and to be expected. What we need for it is the mechanisms in
 place to roll out versions and collect feedback and make sure the code is
 stable.
 
 As a matter of fact, I didn't move the Ensure all the necessary files are
 in place... task only because it was already started. But really it's
 something to do post-tech preview, so I'd welcome it put on the side for
 now and considered for the final. It's OK if we don't have README files
 everywhere for the tech preview.
 
 Please let's not disperse and push toward the tech preview which is almost
 there. The (badly needed) polishing is for the months after that tech
 preview.

Many people are surprised by the lack of camelcase forwarding headers in the 
current frameworks (unless depending on kde4support). Shouldn't we have that 
in the tech preview? Otherwise people can't even use the tech preview for 
anything, especially in the case of existing code, leading to commits like 
change all camelcase includes to lowercase includes (as happened recently in 
kate).

This seems to me like a pre-requisite for any kind of release -- and 
especially a final release for KArchive and ThreadWeaver.

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE, in particular KDE Frameworks 5

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: kde4support: kstandarddirs_unittest fix

2013-12-28 Thread David Faure
On Monday 23 December 2013 14:51:19 Alex Merry wrote:
 If $PREFIX/share/applications does not exist, KStandardDirs will not add
 it.  It will then notice that nothing matching the installation prefix
 was added, and add installPath(xdgdata-apps), which is
 $PREFIX/share/applications/kde5/.

OK, sounds like that bit of code is broken and should have added 
share/applications instead, i.e. NOT simply installPath().

 However, if $PREFIX/share/applications *does* exist, it will add it
 (regardless of XDG_DATA_DIRS, because it *always* considers
 installPath(kdedir)/share - as well as each $KDEDIRS/share - to be in
 XDG_DATA_DIRS), and then *not* add installPath(xdgdata-apps), because
 it determines the installation prefix to have already been dealt with.
 
 Now, a comment in kstandarddirs.cpp (line 1870) says that the latter
 case is the correct behaviour; I'm guessing that when you wrote that
 (2008), you didn't anticipate the possibility that the installation
 directory might not contain share/applications, since KIO installs a
 couple of desktop files.

Yeah, it's supposed to just always exist.

 I guess a possible solution is to change the return value of
 installPath(xdgdata-apps) to not include the trailing kde5.

Yes, that sounds like the right fix. It will not change were we actually 
install the files, it simply returns the base dir for the installation path.

 Actually, I think we should just install application files directly in
 share/applications. (ie: change the value of XDG_APPS_INSTALL_DIR in
 KDEInstallDirs.cmake).  It makes everyone's lives more difficult to have
 subdirectories (as I discovered while creating and implementing the
 MPRIS2 spec - how do you tell applications how to find Amarok's desktop
 file: amarok.desktop won't work, because it's not in
 share/applications, but kde4-amarok.desktop or kde/amarok.desktop
 requires knowing something about the buildsystem magic that installs the
 desktop file).

Ah, if only Waldo was still here. I'm not sure exactly what the reasoning for 
subdirs under applications was. There's different methods in KService and 
KToolInvocation for dealing with this, i.e. if you had used 
findServiceByDesktopName rather than ByDesktopPath it should have worked iirc.
But yeah - namespacing of desktop files doesn't sound that useful, when the 
binaries are all in the same bin dir anyway.

I just hope I'm not overlooking something by giving you a green light for 
changing this, because I wasn't the author of the original decision.

But yeah, go ahead, I can't think of a reason why not.

Maybe this was related to the idea of having some binaries in libexec (which 
can therefore be versionned), but we're not doing this (after other DEs 
complained), and we can just append a '5' (to both the executable name and the 
desktop file name) for the same effect.

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE, in particular KDE Frameworks 5

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Forward includes

2013-12-28 Thread David Faure
On Friday 27 December 2013 19:54:14 Àlex Fiestas wrote:
 On Friday 27 December 2013 19:00:14 Aleix Pol wrote:
  Hi,
  I've been going through the kde4support forward includes, since I wanted
  to
  start making the modules I decided we'd better make sure all of them are
  working properly.
  
  After some research, I found that I don't have these available, can
  somebody please tell me if I'm missing some dependency or if these are
  indeed not available [1]? If there's no problem with this, I'll proceed to
  change these for a warning that they're not available anymore (see
  attachment).

 I have checked a few of them manually and indeed the real header files do
 not exists.

Yes, but for at least one of them it's a mistake:
I forgot to add the rule for installing kdbusservicestarter.h
Will fix asap.

The others look like stuff that got removed, or was made internal.
 
 The approach in the patch seems correct, we don't want to add a source
 incompatible change by removing them, but I'd change the message from:
 
 #warning This file is not available anymore in KF5, please see
 http://community.kde.org/Frameworks/Porting_Notes

I think you should use #error instead.
#warning doesn't work with MSVC iirc (it uses #pragma message(..) instead).

Well, #pragma message() works with gcc, so you could use that everywhere if 
you don't want to make it an error.

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE, in particular KDE Frameworks 5

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: What are the plans with CamelCase includes?

2013-12-28 Thread David Faure
On Friday 27 December 2013 19:01:09 Friedrich W. H. Kossebau wrote:
 So existing code ported from kdelibs would have to explicitely prefix the 
 includes, e.g. be changed like
 #include KLocalizedString - #include KI18n/KLocalizedString

Definitely don't want this.

See the QtGui/QLabel - QtWidgets/QLabel issue between Qt4 and Qt5, we don't 
want to create the same problems with KDE Frameworks.

So yeah, I'm definitely in favour of *Targets.cmake files,
should include the path including the module prefix

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE, in particular KDE Frameworks 5

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: XDG_APPS_INSTALL_DIR and KStandardDirs

2013-12-28 Thread David Faure
On Tuesday 24 December 2013 15:46:34 Alex Merry wrote:
 This got a bit lost in the other thread on the kde4support tests...

Ah. I think I unconfused it a few minutes ago, didn't see this.

 I want to advocate setting XDG_APPS_INSTALL_DIR to be
 $CMAKE_INSTALL_PREFIX/share/applications instead of
 $CMAKE_INSTALL_PREFIX/share/applications/kde5.
 
 I think we should be putting our application desktop files directory in
 share/applications, not in share/applications/kde5.

I agree, but not for the reason below.
The reason below can be fixed by just fixing KStandardDirs.
But indeed we can make our lives easier by just using share/applications 
everywhere, like everyone else does.

David.

 The reason this has come up is that KStandardDirs currently contains a
 hack for how it compiles the value of xdgdata-apps.  The upshot of this
 is that if the directory $CMAKE_INSTALL_PREFIX/share/applications does
 not exist, KStandardDirs::resourceDirs(xdgdata-apps) does not contain
 that directory, but instead contains XDG_APPS_INSTALL_DIR, which is
 $CMAKE_INSTALL_PREFIX/share/applications/kde5.
 
 In normal circumstances this does not happen (because the directory
 normally does exist, and if it doesn't it's kind of irrelevant), but it
 does cause one of the unit test to fail on Jenkins.  Changing
 XDG_APPS_INSTALL_DIR would fix it, but it is not the only way to fix it.
  I just think that XDG_APPS_INSTALL_DIR *should* have a different value.
 
 Alex
 ___
 Kde-frameworks-devel mailing list
 Kde-frameworks-devel@kde.org
 https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE, in particular KDE Frameworks 5

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Frameworks repositories on reviewboard

2013-12-28 Thread Kevin Ottens
Hello all,

Everything is in the subject of this email. Thanks to Ben for doing the grunt 
work as usual. Ben you rock!

So now people can report bugs on bugzilla and can send us patches through 
reviewboard.

Cheers!
-- 
Kévin Ottens, http://ervin.ipsquad.net

KDAB - proud supporter of KDE, http://www.kdab.com



signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Refocusing the tech preview

2013-12-28 Thread Kevin Ottens
On Saturday 28 December 2013 11:39:50 David Faure wrote:
 On Saturday 21 December 2013 12:42:04 Kevin Ottens wrote:
  Hello people,
  
  I see that new tasks appeared in the wiki, that's great, they were clearly
  missing. That said, they were all earmarked for the tech preview, so I
  took
  the liberty to move most of them to the list about the final release. We
  should get the tech preview out of the door ASAP now. It won't be perfect,
  that's fine and to be expected. What we need for it is the mechanisms in
  place to roll out versions and collect feedback and make sure the code is
  stable.
  
  As a matter of fact, I didn't move the Ensure all the necessary files are
  in place... task only because it was already started. But really it's
  something to do post-tech preview, so I'd welcome it put on the side for
  now and considered for the final. It's OK if we don't have README files
  everywhere for the tech preview.
  
  Please let's not disperse and push toward the tech preview which is almost
  there. The (badly needed) polishing is for the months after that tech
  preview.
 
 Many people are surprised by the lack of camelcase forwarding headers in the
 current frameworks (unless depending on kde4support). Shouldn't we have
 that in the tech preview? Otherwise people can't even use the tech preview
 for anything, especially in the case of existing code, leading to commits
 like change all camelcase includes to lowercase includes (as happened
 recently in kate).

Well, they could keep the dependency on kde4support just for those headers. 
But I see what you mean... OK, let's put it back in, but it'll need a strong 
push from everyone otherwise we'll get the TP1 slipping even more which I 
really don't like.

 This seems to me like a pre-requisite for any kind of release -- and
 especially a final release for KArchive and ThreadWeaver.

Well, let's not fool ourselves. They're not final releases. We'll have new 
iterations for those until 5.0 is released.

Regards.
-- 
Kévin Ottens, http://ervin.ipsquad.net

KDAB - proud supporter of KDE, http://www.kdab.com



signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: What are the plans with CamelCase includes?

2013-12-28 Thread Kevin Ottens
On Saturday 28 December 2013 11:55:56 David Faure wrote:
 On Friday 27 December 2013 19:01:09 Friedrich W. H. Kossebau wrote:
  So existing code ported from kdelibs would have to explicitely prefix the
  includes, e.g. be changed like
  #include KLocalizedString - #include KI18n/KLocalizedString
 
 Definitely don't want this.
 
 See the QtGui/QLabel - QtWidgets/QLabel issue between Qt4 and Qt5, we don't
 want to create the same problems with KDE Frameworks.
 
 So yeah, I'm definitely in favour of *Targets.cmake files,
 should include the path including the module prefix

Which is supposed to be the case as discussed in the thread with Aurélien a 
while ago.

The complete consensus was IIRC:
 - For frameworks with K* prefixed classes, the include line should be 
#include KFoo;
 - For frameworks not using the K* prefix for their classes (and generally 
using namespaces) the include line should be #include Framework/Foo.

With the forward headers included, if the frameworks doesn't behave like that 
it should be considered a bug.

Cheers.
-- 
Kévin Ottens, http://ervin.ipsquad.net

KDAB - proud supporter of KDE, http://www.kdab.com



signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Review Request 114693: fix KFileWidget url selection

2013-12-28 Thread Michal Humpula

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

Review request for KDE Frameworks.


Repository: kio


Description
---

If I understand correctly documentation of KFileWidget, it should be perfectly 
ok to do something like this:

KEncodingFileDialog::getOpenUrlsAndEncoding(QString(), 
QUrl(file:///etc/passwd));

But that doesn't display the thing I'm expecting. Tracing it down I came up 
with the fix. I'm not claiming that it's the correct one, but at least in my 
situation the KFileWidget behaves as expected in all tested situations. 

Surprisingly the

void KFileWidgetPrivate::setLocationText(const QListQUrl urlList)

doesn't call any setUrl, which hints that it could actually be correct.


Diffs
-

  src/filewidgets/kfilewidget.cpp 11597b3 

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


Testing
---


Thanks,

Michal Humpula

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 114697: KStandardDirs: special case for xdgdata-apps

2013-12-28 Thread David Faure

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

Ship it!


Thanks.

- David Faure


On Dec. 28, 2013, 12:35 p.m., Alex Merry wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/114697/
 ---
 
 (Updated Dec. 28, 2013, 12:35 p.m.)
 
 
 Review request for KDE Frameworks and David Faure.
 
 
 Repository: kde4support
 
 
 Description
 ---
 
 For resourceDirs(xdgdata-apps), we do not use
 installPath(xdgdata-apps) as the fallback (which is probably a
 subdirectory of CMAKE_INSTALL_PREFIX/share/applications/), but instead
 use CMAKE_INSTALL_PREFIX/share/applications/ directly.
 
 In a sensible installation, this should make no difference, because if
 XDG_DATA_DIRS is set to include CMAKE_INSTALL_PREFIX/share, and that
 directory contains an applications directory, the result of
 installPath(xdgdata-apps) is never used.  However, this will (a) act
 as a bit of a safety blanket for people who set XDG_DATA_DIRS wrong and
 (b) allow the tests to pass on Jenkins, which installs kde4support to
 its own directory (which then does not have a share/applications
 subdirectory).
 
 
 Diffs
 -
 
   src/kdecore/kstandarddirs.cpp f9a29cd0e1c91861afe2a47eacac2f006248af54 
 
 Diff: https://git.reviewboard.kde.org/r/114697/diff/
 
 
 Testing
 ---
 
 The kstandarddirs unit test now passes whether or not 
 CMAKE_INSTALL_PREFIX/share/applications exists or not.
 
 
 Thanks,
 
 Alex Merry
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Review Request 114698: Make KStandardDirsTest work when not installed or KDEDIRS not set correctly

2013-12-28 Thread Alex Merry

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

Review request for KDE Frameworks and David Faure.


Repository: kde4support


Description
---

Prevent KStandardDirsTest::testThreads producing multiple results

testThreads() calls several other tests, some of which can call QSKIP.
This means that one test can produce several SKIPs, which doesn't make
much sense.  So, instead, we check the SKIP conditions in testThreads()
itself.

Remove duplicate test code

These lines exist in another test

Use data directories we install ourselves in kstandarddirs_unittests

That way, we know they will be there, even when KDEDIRS is not set
correctly or another framework changes the data files it installs.

Skip tests requiring KConfig if KDEDIRS is not set correctly

If frameworks are installed to different locations, KDEDIRS needs to be
set appropriately for KStandardDirs to work.  kstandarddirs_unittest now
checks for this, and skips the relevant tests with a helpful message if
KDEDIRS is not set properly.

Skip tests that require kde4support to be installed if it is not

Various KStandardDirs tests depend on kde4support being installed (since
KStandardDirs is supposed to find installed things), so we cannot
sensibly run them if kde4support has not been installed yet.


Diffs
-

  autotests/kstandarddirstest.h fb1058d9a9e7440a8aa012ed0da86f40ae3ad01e 
  autotests/kstandarddirstest.cpp 5e0151b1cf7afe344d5339031bca77405ebeb3a5 
  src/CMakeLists.txt 1529d0a33d9b80a6e5f937783ee7b8a0145d284a 
  src/config-kstandarddirs.h.cmake 40994a9af1675af2a0edb89cdd95352e84227f73 

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


Testing
---

Tests pass (or at least skip with a helpful message) when kde4support is not 
installed, or installed to its own prefix with KDEDIRS set or unset (or set to 
only include the kde4support directory, or only the rest of the frameworks).  
Also still pass when kde4support is installed to the same prefix as the other 
frameworks.


Thanks,

Alex Merry

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 114697: KStandardDirs: special case for xdgdata-apps

2013-12-28 Thread Commit Hook

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


This review has been submitted with commit 
deaa35d72fe0c6248293b5836e084fe8ade2ec0d by Alex Merry to branch master.

- Commit Hook


On Dec. 28, 2013, 12:35 p.m., Alex Merry wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/114697/
 ---
 
 (Updated Dec. 28, 2013, 12:35 p.m.)
 
 
 Review request for KDE Frameworks and David Faure.
 
 
 Repository: kde4support
 
 
 Description
 ---
 
 For resourceDirs(xdgdata-apps), we do not use
 installPath(xdgdata-apps) as the fallback (which is probably a
 subdirectory of CMAKE_INSTALL_PREFIX/share/applications/), but instead
 use CMAKE_INSTALL_PREFIX/share/applications/ directly.
 
 In a sensible installation, this should make no difference, because if
 XDG_DATA_DIRS is set to include CMAKE_INSTALL_PREFIX/share, and that
 directory contains an applications directory, the result of
 installPath(xdgdata-apps) is never used.  However, this will (a) act
 as a bit of a safety blanket for people who set XDG_DATA_DIRS wrong and
 (b) allow the tests to pass on Jenkins, which installs kde4support to
 its own directory (which then does not have a share/applications
 subdirectory).
 
 
 Diffs
 -
 
   src/kdecore/kstandarddirs.cpp f9a29cd0e1c91861afe2a47eacac2f006248af54 
 
 Diff: https://git.reviewboard.kde.org/r/114697/diff/
 
 
 Testing
 ---
 
 The kstandarddirs unit test now passes whether or not 
 CMAKE_INSTALL_PREFIX/share/applications exists or not.
 
 
 Thanks,
 
 Alex Merry
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 114698: Make KStandardDirsTest work when not installed or KDEDIRS not set correctly

2013-12-28 Thread David Faure

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

Ship it!


Ship It!

- David Faure


On Dec. 28, 2013, 12:39 p.m., Alex Merry wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/114698/
 ---
 
 (Updated Dec. 28, 2013, 12:39 p.m.)
 
 
 Review request for KDE Frameworks and David Faure.
 
 
 Repository: kde4support
 
 
 Description
 ---
 
 Prevent KStandardDirsTest::testThreads producing multiple results
 
 testThreads() calls several other tests, some of which can call QSKIP.
 This means that one test can produce several SKIPs, which doesn't make
 much sense.  So, instead, we check the SKIP conditions in testThreads()
 itself.
 
 Remove duplicate test code
 
 These lines exist in another test
 
 Use data directories we install ourselves in kstandarddirs_unittests
 
 That way, we know they will be there, even when KDEDIRS is not set
 correctly or another framework changes the data files it installs.
 
 Skip tests requiring KConfig if KDEDIRS is not set correctly
 
 If frameworks are installed to different locations, KDEDIRS needs to be
 set appropriately for KStandardDirs to work.  kstandarddirs_unittest now
 checks for this, and skips the relevant tests with a helpful message if
 KDEDIRS is not set properly.
 
 Skip tests that require kde4support to be installed if it is not
 
 Various KStandardDirs tests depend on kde4support being installed (since
 KStandardDirs is supposed to find installed things), so we cannot
 sensibly run them if kde4support has not been installed yet.
 
 
 Diffs
 -
 
   autotests/kstandarddirstest.h fb1058d9a9e7440a8aa012ed0da86f40ae3ad01e 
   autotests/kstandarddirstest.cpp 5e0151b1cf7afe344d5339031bca77405ebeb3a5 
   src/CMakeLists.txt 1529d0a33d9b80a6e5f937783ee7b8a0145d284a 
   src/config-kstandarddirs.h.cmake 40994a9af1675af2a0edb89cdd95352e84227f73 
 
 Diff: https://git.reviewboard.kde.org/r/114698/diff/
 
 
 Testing
 ---
 
 Tests pass (or at least skip with a helpful message) when kde4support is not 
 installed, or installed to its own prefix with KDEDIRS set or unset (or set 
 to only include the kde4support directory, or only the rest of the 
 frameworks).  Also still pass when kde4support is installed to the same 
 prefix as the other frameworks.
 
 
 Thanks,
 
 Alex Merry
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 114697: KStandardDirs: special case for xdgdata-apps

2013-12-28 Thread Alex Merry

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

(Updated Dec. 28, 2013, 12:42 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and David Faure.


Repository: kde4support


Description
---

For resourceDirs(xdgdata-apps), we do not use
installPath(xdgdata-apps) as the fallback (which is probably a
subdirectory of CMAKE_INSTALL_PREFIX/share/applications/), but instead
use CMAKE_INSTALL_PREFIX/share/applications/ directly.

In a sensible installation, this should make no difference, because if
XDG_DATA_DIRS is set to include CMAKE_INSTALL_PREFIX/share, and that
directory contains an applications directory, the result of
installPath(xdgdata-apps) is never used.  However, this will (a) act
as a bit of a safety blanket for people who set XDG_DATA_DIRS wrong and
(b) allow the tests to pass on Jenkins, which installs kde4support to
its own directory (which then does not have a share/applications
subdirectory).


Diffs
-

  src/kdecore/kstandarddirs.cpp f9a29cd0e1c91861afe2a47eacac2f006248af54 

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


Testing
---

The kstandarddirs unit test now passes whether or not 
CMAKE_INSTALL_PREFIX/share/applications exists or not.


Thanks,

Alex Merry

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 114698: Make KStandardDirsTest work when not installed or KDEDIRS not set correctly

2013-12-28 Thread Commit Hook

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


This review has been submitted with commit 
6da2ebe9231d8bc995dcd5742889a7e055ee90db by Alex Merry to branch master.

- Commit Hook


On Dec. 28, 2013, 12:39 p.m., Alex Merry wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/114698/
 ---
 
 (Updated Dec. 28, 2013, 12:39 p.m.)
 
 
 Review request for KDE Frameworks and David Faure.
 
 
 Repository: kde4support
 
 
 Description
 ---
 
 Prevent KStandardDirsTest::testThreads producing multiple results
 
 testThreads() calls several other tests, some of which can call QSKIP.
 This means that one test can produce several SKIPs, which doesn't make
 much sense.  So, instead, we check the SKIP conditions in testThreads()
 itself.
 
 Remove duplicate test code
 
 These lines exist in another test
 
 Use data directories we install ourselves in kstandarddirs_unittests
 
 That way, we know they will be there, even when KDEDIRS is not set
 correctly or another framework changes the data files it installs.
 
 Skip tests requiring KConfig if KDEDIRS is not set correctly
 
 If frameworks are installed to different locations, KDEDIRS needs to be
 set appropriately for KStandardDirs to work.  kstandarddirs_unittest now
 checks for this, and skips the relevant tests with a helpful message if
 KDEDIRS is not set properly.
 
 Skip tests that require kde4support to be installed if it is not
 
 Various KStandardDirs tests depend on kde4support being installed (since
 KStandardDirs is supposed to find installed things), so we cannot
 sensibly run them if kde4support has not been installed yet.
 
 
 Diffs
 -
 
   autotests/kstandarddirstest.h fb1058d9a9e7440a8aa012ed0da86f40ae3ad01e 
   autotests/kstandarddirstest.cpp 5e0151b1cf7afe344d5339031bca77405ebeb3a5 
   src/CMakeLists.txt 1529d0a33d9b80a6e5f937783ee7b8a0145d284a 
   src/config-kstandarddirs.h.cmake 40994a9af1675af2a0edb89cdd95352e84227f73 
 
 Diff: https://git.reviewboard.kde.org/r/114698/diff/
 
 
 Testing
 ---
 
 Tests pass (or at least skip with a helpful message) when kde4support is not 
 installed, or installed to its own prefix with KDEDIRS set or unset (or set 
 to only include the kde4support directory, or only the rest of the 
 frameworks).  Also still pass when kde4support is installed to the same 
 prefix as the other frameworks.
 
 
 Thanks,
 
 Alex Merry
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 114698: Make KStandardDirsTest work when not installed or KDEDIRS not set correctly

2013-12-28 Thread Commit Hook

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


This review has been submitted with commit 
05c1fd6f82995796a9c446f6309e92e16d63717f by Alex Merry to branch master.

- Commit Hook


On Dec. 28, 2013, 12:39 p.m., Alex Merry wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/114698/
 ---
 
 (Updated Dec. 28, 2013, 12:39 p.m.)
 
 
 Review request for KDE Frameworks and David Faure.
 
 
 Repository: kde4support
 
 
 Description
 ---
 
 Prevent KStandardDirsTest::testThreads producing multiple results
 
 testThreads() calls several other tests, some of which can call QSKIP.
 This means that one test can produce several SKIPs, which doesn't make
 much sense.  So, instead, we check the SKIP conditions in testThreads()
 itself.
 
 Remove duplicate test code
 
 These lines exist in another test
 
 Use data directories we install ourselves in kstandarddirs_unittests
 
 That way, we know they will be there, even when KDEDIRS is not set
 correctly or another framework changes the data files it installs.
 
 Skip tests requiring KConfig if KDEDIRS is not set correctly
 
 If frameworks are installed to different locations, KDEDIRS needs to be
 set appropriately for KStandardDirs to work.  kstandarddirs_unittest now
 checks for this, and skips the relevant tests with a helpful message if
 KDEDIRS is not set properly.
 
 Skip tests that require kde4support to be installed if it is not
 
 Various KStandardDirs tests depend on kde4support being installed (since
 KStandardDirs is supposed to find installed things), so we cannot
 sensibly run them if kde4support has not been installed yet.
 
 
 Diffs
 -
 
   autotests/kstandarddirstest.h fb1058d9a9e7440a8aa012ed0da86f40ae3ad01e 
   autotests/kstandarddirstest.cpp 5e0151b1cf7afe344d5339031bca77405ebeb3a5 
   src/CMakeLists.txt 1529d0a33d9b80a6e5f937783ee7b8a0145d284a 
   src/config-kstandarddirs.h.cmake 40994a9af1675af2a0edb89cdd95352e84227f73 
 
 Diff: https://git.reviewboard.kde.org/r/114698/diff/
 
 
 Testing
 ---
 
 Tests pass (or at least skip with a helpful message) when kde4support is not 
 installed, or installed to its own prefix with KDEDIRS set or unset (or set 
 to only include the kde4support directory, or only the rest of the 
 frameworks).  Also still pass when kde4support is installed to the same 
 prefix as the other frameworks.
 
 
 Thanks,
 
 Alex Merry
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 114698: Make KStandardDirsTest work when not installed or KDEDIRS not set correctly

2013-12-28 Thread Commit Hook

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


This review has been submitted with commit 
5aaeabc5c62f902a601a3f772cd56cda478b0cfa by Alex Merry to branch master.

- Commit Hook


On Dec. 28, 2013, 12:39 p.m., Alex Merry wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/114698/
 ---
 
 (Updated Dec. 28, 2013, 12:39 p.m.)
 
 
 Review request for KDE Frameworks and David Faure.
 
 
 Repository: kde4support
 
 
 Description
 ---
 
 Prevent KStandardDirsTest::testThreads producing multiple results
 
 testThreads() calls several other tests, some of which can call QSKIP.
 This means that one test can produce several SKIPs, which doesn't make
 much sense.  So, instead, we check the SKIP conditions in testThreads()
 itself.
 
 Remove duplicate test code
 
 These lines exist in another test
 
 Use data directories we install ourselves in kstandarddirs_unittests
 
 That way, we know they will be there, even when KDEDIRS is not set
 correctly or another framework changes the data files it installs.
 
 Skip tests requiring KConfig if KDEDIRS is not set correctly
 
 If frameworks are installed to different locations, KDEDIRS needs to be
 set appropriately for KStandardDirs to work.  kstandarddirs_unittest now
 checks for this, and skips the relevant tests with a helpful message if
 KDEDIRS is not set properly.
 
 Skip tests that require kde4support to be installed if it is not
 
 Various KStandardDirs tests depend on kde4support being installed (since
 KStandardDirs is supposed to find installed things), so we cannot
 sensibly run them if kde4support has not been installed yet.
 
 
 Diffs
 -
 
   autotests/kstandarddirstest.h fb1058d9a9e7440a8aa012ed0da86f40ae3ad01e 
   autotests/kstandarddirstest.cpp 5e0151b1cf7afe344d5339031bca77405ebeb3a5 
   src/CMakeLists.txt 1529d0a33d9b80a6e5f937783ee7b8a0145d284a 
   src/config-kstandarddirs.h.cmake 40994a9af1675af2a0edb89cdd95352e84227f73 
 
 Diff: https://git.reviewboard.kde.org/r/114698/diff/
 
 
 Testing
 ---
 
 Tests pass (or at least skip with a helpful message) when kde4support is not 
 installed, or installed to its own prefix with KDEDIRS set or unset (or set 
 to only include the kde4support directory, or only the rest of the 
 frameworks).  Also still pass when kde4support is installed to the same 
 prefix as the other frameworks.
 
 
 Thanks,
 
 Alex Merry
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 114698: Make KStandardDirsTest work when not installed or KDEDIRS not set correctly

2013-12-28 Thread Alex Merry

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

(Updated Dec. 28, 2013, 12:48 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and David Faure.


Repository: kde4support


Description
---

Prevent KStandardDirsTest::testThreads producing multiple results

testThreads() calls several other tests, some of which can call QSKIP.
This means that one test can produce several SKIPs, which doesn't make
much sense.  So, instead, we check the SKIP conditions in testThreads()
itself.

Remove duplicate test code

These lines exist in another test

Use data directories we install ourselves in kstandarddirs_unittests

That way, we know they will be there, even when KDEDIRS is not set
correctly or another framework changes the data files it installs.

Skip tests requiring KConfig if KDEDIRS is not set correctly

If frameworks are installed to different locations, KDEDIRS needs to be
set appropriately for KStandardDirs to work.  kstandarddirs_unittest now
checks for this, and skips the relevant tests with a helpful message if
KDEDIRS is not set properly.

Skip tests that require kde4support to be installed if it is not

Various KStandardDirs tests depend on kde4support being installed (since
KStandardDirs is supposed to find installed things), so we cannot
sensibly run them if kde4support has not been installed yet.


Diffs
-

  autotests/kstandarddirstest.h fb1058d9a9e7440a8aa012ed0da86f40ae3ad01e 
  autotests/kstandarddirstest.cpp 5e0151b1cf7afe344d5339031bca77405ebeb3a5 
  src/CMakeLists.txt 1529d0a33d9b80a6e5f937783ee7b8a0145d284a 
  src/config-kstandarddirs.h.cmake 40994a9af1675af2a0edb89cdd95352e84227f73 

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


Testing
---

Tests pass (or at least skip with a helpful message) when kde4support is not 
installed, or installed to its own prefix with KDEDIRS set or unset (or set to 
only include the kde4support directory, or only the rest of the frameworks).  
Also still pass when kde4support is installed to the same prefix as the other 
frameworks.


Thanks,

Alex Merry

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 114698: Make KStandardDirsTest work when not installed or KDEDIRS not set correctly

2013-12-28 Thread Commit Hook

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


This review has been submitted with commit 
a51f0ab2c569d9a46e25ea4f4e6bef90ec3c44c3 by Alex Merry to branch master.

- Commit Hook


On Dec. 28, 2013, 12:48 p.m., Alex Merry wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/114698/
 ---
 
 (Updated Dec. 28, 2013, 12:48 p.m.)
 
 
 Review request for KDE Frameworks and David Faure.
 
 
 Repository: kde4support
 
 
 Description
 ---
 
 Prevent KStandardDirsTest::testThreads producing multiple results
 
 testThreads() calls several other tests, some of which can call QSKIP.
 This means that one test can produce several SKIPs, which doesn't make
 much sense.  So, instead, we check the SKIP conditions in testThreads()
 itself.
 
 Remove duplicate test code
 
 These lines exist in another test
 
 Use data directories we install ourselves in kstandarddirs_unittests
 
 That way, we know they will be there, even when KDEDIRS is not set
 correctly or another framework changes the data files it installs.
 
 Skip tests requiring KConfig if KDEDIRS is not set correctly
 
 If frameworks are installed to different locations, KDEDIRS needs to be
 set appropriately for KStandardDirs to work.  kstandarddirs_unittest now
 checks for this, and skips the relevant tests with a helpful message if
 KDEDIRS is not set properly.
 
 Skip tests that require kde4support to be installed if it is not
 
 Various KStandardDirs tests depend on kde4support being installed (since
 KStandardDirs is supposed to find installed things), so we cannot
 sensibly run them if kde4support has not been installed yet.
 
 
 Diffs
 -
 
   autotests/kstandarddirstest.h fb1058d9a9e7440a8aa012ed0da86f40ae3ad01e 
   autotests/kstandarddirstest.cpp 5e0151b1cf7afe344d5339031bca77405ebeb3a5 
   src/CMakeLists.txt 1529d0a33d9b80a6e5f937783ee7b8a0145d284a 
   src/config-kstandarddirs.h.cmake 40994a9af1675af2a0edb89cdd95352e84227f73 
 
 Diff: https://git.reviewboard.kde.org/r/114698/diff/
 
 
 Testing
 ---
 
 Tests pass (or at least skip with a helpful message) when kde4support is not 
 installed, or installed to its own prefix with KDEDIRS set or unset (or set 
 to only include the kde4support directory, or only the rest of the 
 frameworks).  Also still pass when kde4support is installed to the same 
 prefix as the other frameworks.
 
 
 Thanks,
 
 Alex Merry
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Review Request 114699: Use enums instead of ints in method types

2013-12-28 Thread Alex Merry

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

Review request for KDE Frameworks.


Repository: kmediaplayer


Description
---

Use enums instead of ints in method types.

This adds to the porting effort a little (but not in many places), but gives 
more type-safety.


Diffs
-

  autotests/playertest.cpp 638e86fecf7eb418c802b1f46d04e864d8e161f6 
  autotests/testplayer.h 285da38fdf886ec8710e3a0d595ca20777426f86 
  autotests/viewtest.cpp 337ac3eef735dab7860bb6577c72fa33d3597068 
  src/kmediaplayer/player.h 57935bae537dba355b4c0cf2306dd6c993762661 
  src/kmediaplayer/player.cpp c0be47666094a12abc160538798efafe1e3e1fb0 
  src/kmediaplayer/view.h c3af06b51202b29f93ca0e3509ad41b8bbb91b0a 
  src/kmediaplayer/view.cpp 51e4c4ba5b16a4d7b90e57f78281fedb524eca54 

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


Testing
---

Builds, tests run.


Thanks,

Alex Merry

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Review Request 114700: Add -DKCOREADDONS_LIB to the exported defines for KF5::CoreAddons

2013-12-28 Thread Alex Merry

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

Review request for KDE Frameworks.


Repository: kcoreaddons


Description
---

Add -DKCOREADDONS_LIB to the exported defines for KF5::CoreAddons

This allows code (specifically, kimagecache.h in kguiaddons) to check
whether KF5::CoreAddons has been linked against.


In theory, we could apply this across the frameworks, in much the same way that 
Qt appears to do (it adds -DQT_CORE_LIB, -DQT_GUI_LIB etc etc).


Diffs
-

  src/lib/CMakeLists.txt 0e18f42b37055e7aa5b686ec72b1906a16512b80 

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


Testing
---

KCoreAddons and things that depend on it still build.


Thanks,

Alex Merry

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Review Request 114701: Print a warning if kimagecache.h is used without KF5::CoreAddons

2013-12-28 Thread Alex Merry

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

Review request for KDE Frameworks.


Repository: kguiaddons


Description
---

Print a warning if kimagecache.h is used without KF5::CoreAddons

If an application links against KF5::GuiAddons but not KF5::CoreAddons,
they cannot use KImageCache.  So we use defines (set by the exported
targets) to check whether the current target has been linked against
KF5::CoreAddons, and print a warning if not (to explain the expected
kshareddatacache.h not found error).


Diffs
-

  src/CMakeLists.txt d6ee3d9987319f9a3d1f8b2f66e00d2ec4bda1e5 
  src/util/kimagecache.h 007891c757de9bedc5cf0c3b734e250004727ec4 

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


Testing
---

plasma-framework (which uses KImageCache) still builds fine.  If an older 
version of KCoreAddons (without the KCOREADDONS_LIB define) is installed, a 
warning is printed when kimagecache.h is included.  If the patch in review 
114700 is applied to KCoreAddons, the warning is not printed.


Thanks,

Alex Merry

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 114700: Add -DKCOREADDONS_LIB to the exported defines for KF5::CoreAddons

2013-12-28 Thread David Faure

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


OK for this particular case, but in general I don't like Qt's -DQT_CORE_LIB 
very much because it is buildsystem dependent (i.e. it makes it more 
complicated to use a different buildsystem for the application than the one 
used by the library - qmake in Qt's case, cmake in our case).

- David Faure


On Dec. 28, 2013, 1:03 p.m., Alex Merry wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/114700/
 ---
 
 (Updated Dec. 28, 2013, 1:03 p.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Repository: kcoreaddons
 
 
 Description
 ---
 
 Add -DKCOREADDONS_LIB to the exported defines for KF5::CoreAddons
 
 This allows code (specifically, kimagecache.h in kguiaddons) to check
 whether KF5::CoreAddons has been linked against.
 
 
 In theory, we could apply this across the frameworks, in much the same way 
 that Qt appears to do (it adds -DQT_CORE_LIB, -DQT_GUI_LIB etc etc).
 
 
 Diffs
 -
 
   src/lib/CMakeLists.txt 0e18f42b37055e7aa5b686ec72b1906a16512b80 
 
 Diff: https://git.reviewboard.kde.org/r/114700/diff/
 
 
 Testing
 ---
 
 KCoreAddons and things that depend on it still build.
 
 
 Thanks,
 
 Alex Merry
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 114701: Print a warning if kimagecache.h is used without KF5::CoreAddons

2013-12-28 Thread David Faure

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


See, this will print an unnecessary (i.e. wrong) warning in case we *are* 
linking to KCoreAddons, but not using cmake.
qmake users will have to add a define just to silence the warning. So this 
create more work, rather than helping...
We'd have to ifdef the whole thing with a if we are using cmake define

- David Faure


On Dec. 28, 2013, 1:03 p.m., Alex Merry wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/114701/
 ---
 
 (Updated Dec. 28, 2013, 1:03 p.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Repository: kguiaddons
 
 
 Description
 ---
 
 Print a warning if kimagecache.h is used without KF5::CoreAddons
 
 If an application links against KF5::GuiAddons but not KF5::CoreAddons,
 they cannot use KImageCache.  So we use defines (set by the exported
 targets) to check whether the current target has been linked against
 KF5::CoreAddons, and print a warning if not (to explain the expected
 kshareddatacache.h not found error).
 
 
 Diffs
 -
 
   src/CMakeLists.txt d6ee3d9987319f9a3d1f8b2f66e00d2ec4bda1e5 
   src/util/kimagecache.h 007891c757de9bedc5cf0c3b734e250004727ec4 
 
 Diff: https://git.reviewboard.kde.org/r/114701/diff/
 
 
 Testing
 ---
 
 plasma-framework (which uses KImageCache) still builds fine.  If an older 
 version of KCoreAddons (without the KCOREADDONS_LIB define) is installed, a 
 warning is printed when kimagecache.h is included.  If the patch in review 
 114700 is applied to KCoreAddons, the warning is not printed.
 
 
 Thanks,
 
 Alex Merry
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 114701: Print a warning if kimagecache.h is used without KF5::CoreAddons

2013-12-28 Thread Alex Merry


 On Dec. 28, 2013, 1:19 p.m., David Faure wrote:
  See, this will print an unnecessary (i.e. wrong) warning in case we *are* 
  linking to KCoreAddons, but not using cmake.
  qmake users will have to add a define just to silence the warning. So this 
  create more work, rather than helping...
  We'd have to ifdef the whole thing with a if we are using cmake define

I deliberately set it up not to print the warning in that case - that's why it 
checks that KCOREADDONS_LIB is not defined AND that KGUIADDONS_LIB is defined.


- Alex


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


On Dec. 28, 2013, 1:03 p.m., Alex Merry wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/114701/
 ---
 
 (Updated Dec. 28, 2013, 1:03 p.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Repository: kguiaddons
 
 
 Description
 ---
 
 Print a warning if kimagecache.h is used without KF5::CoreAddons
 
 If an application links against KF5::GuiAddons but not KF5::CoreAddons,
 they cannot use KImageCache.  So we use defines (set by the exported
 targets) to check whether the current target has been linked against
 KF5::CoreAddons, and print a warning if not (to explain the expected
 kshareddatacache.h not found error).
 
 
 Diffs
 -
 
   src/CMakeLists.txt d6ee3d9987319f9a3d1f8b2f66e00d2ec4bda1e5 
   src/util/kimagecache.h 007891c757de9bedc5cf0c3b734e250004727ec4 
 
 Diff: https://git.reviewboard.kde.org/r/114701/diff/
 
 
 Testing
 ---
 
 plasma-framework (which uses KImageCache) still builds fine.  If an older 
 version of KCoreAddons (without the KCOREADDONS_LIB define) is installed, a 
 warning is printed when kimagecache.h is included.  If the patch in review 
 114700 is applied to KCoreAddons, the warning is not printed.
 
 
 Thanks,
 
 Alex Merry
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 114701: Print a warning if kimagecache.h is used without KF5::CoreAddons

2013-12-28 Thread Alex Merry


 On Dec. 28, 2013, 1:42 p.m., David Faure wrote:
  Ah indeed, well done.

Does that mean you're happy for me to ship 114700 as well?


- Alex


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


On Dec. 28, 2013, 1:03 p.m., Alex Merry wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/114701/
 ---
 
 (Updated Dec. 28, 2013, 1:03 p.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Repository: kguiaddons
 
 
 Description
 ---
 
 Print a warning if kimagecache.h is used without KF5::CoreAddons
 
 If an application links against KF5::GuiAddons but not KF5::CoreAddons,
 they cannot use KImageCache.  So we use defines (set by the exported
 targets) to check whether the current target has been linked against
 KF5::CoreAddons, and print a warning if not (to explain the expected
 kshareddatacache.h not found error).
 
 
 Diffs
 -
 
   src/CMakeLists.txt d6ee3d9987319f9a3d1f8b2f66e00d2ec4bda1e5 
   src/util/kimagecache.h 007891c757de9bedc5cf0c3b734e250004727ec4 
 
 Diff: https://git.reviewboard.kde.org/r/114701/diff/
 
 
 Testing
 ---
 
 plasma-framework (which uses KImageCache) still builds fine.  If an older 
 version of KCoreAddons (without the KCOREADDONS_LIB define) is installed, a 
 warning is printed when kimagecache.h is included.  If the patch in review 
 114700 is applied to KCoreAddons, the warning is not printed.
 
 
 Thanks,
 
 Alex Merry
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 114700: Add -DKCOREADDONS_LIB to the exported defines for KF5::CoreAddons

2013-12-28 Thread David Faure

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

Ship it!


Ship It!

- David Faure


On Dec. 28, 2013, 1:03 p.m., Alex Merry wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/114700/
 ---
 
 (Updated Dec. 28, 2013, 1:03 p.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Repository: kcoreaddons
 
 
 Description
 ---
 
 Add -DKCOREADDONS_LIB to the exported defines for KF5::CoreAddons
 
 This allows code (specifically, kimagecache.h in kguiaddons) to check
 whether KF5::CoreAddons has been linked against.
 
 
 In theory, we could apply this across the frameworks, in much the same way 
 that Qt appears to do (it adds -DQT_CORE_LIB, -DQT_GUI_LIB etc etc).
 
 
 Diffs
 -
 
   src/lib/CMakeLists.txt 0e18f42b37055e7aa5b686ec72b1906a16512b80 
 
 Diff: https://git.reviewboard.kde.org/r/114700/diff/
 
 
 Testing
 ---
 
 KCoreAddons and things that depend on it still build.
 
 
 Thanks,
 
 Alex Merry
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 114701: Print a warning if kimagecache.h is used without KF5::CoreAddons

2013-12-28 Thread Commit Hook

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


This review has been submitted with commit 
9262517af279df80a0de181be3a3ed7d7e7ce9c4 by Alex Merry to branch master.

- Commit Hook


On Dec. 28, 2013, 1:03 p.m., Alex Merry wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/114701/
 ---
 
 (Updated Dec. 28, 2013, 1:03 p.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Repository: kguiaddons
 
 
 Description
 ---
 
 Print a warning if kimagecache.h is used without KF5::CoreAddons
 
 If an application links against KF5::GuiAddons but not KF5::CoreAddons,
 they cannot use KImageCache.  So we use defines (set by the exported
 targets) to check whether the current target has been linked against
 KF5::CoreAddons, and print a warning if not (to explain the expected
 kshareddatacache.h not found error).
 
 
 Diffs
 -
 
   src/CMakeLists.txt d6ee3d9987319f9a3d1f8b2f66e00d2ec4bda1e5 
   src/util/kimagecache.h 007891c757de9bedc5cf0c3b734e250004727ec4 
 
 Diff: https://git.reviewboard.kde.org/r/114701/diff/
 
 
 Testing
 ---
 
 plasma-framework (which uses KImageCache) still builds fine.  If an older 
 version of KCoreAddons (without the KCOREADDONS_LIB define) is installed, a 
 warning is printed when kimagecache.h is included.  If the patch in review 
 114700 is applied to KCoreAddons, the warning is not printed.
 
 
 Thanks,
 
 Alex Merry
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 114700: Add -DKCOREADDONS_LIB to the exported defines for KF5::CoreAddons

2013-12-28 Thread Alex Merry

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

(Updated Dec. 28, 2013, 1:52 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks.


Repository: kcoreaddons


Description
---

Add -DKCOREADDONS_LIB to the exported defines for KF5::CoreAddons

This allows code (specifically, kimagecache.h in kguiaddons) to check
whether KF5::CoreAddons has been linked against.


In theory, we could apply this across the frameworks, in much the same way that 
Qt appears to do (it adds -DQT_CORE_LIB, -DQT_GUI_LIB etc etc).


Diffs
-

  src/lib/CMakeLists.txt 0e18f42b37055e7aa5b686ec72b1906a16512b80 

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


Testing
---

KCoreAddons and things that depend on it still build.


Thanks,

Alex Merry

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 114701: Print a warning if kimagecache.h is used without KF5::CoreAddons

2013-12-28 Thread Alex Merry

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

(Updated Dec. 28, 2013, 1:53 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks.


Repository: kguiaddons


Description
---

Print a warning if kimagecache.h is used without KF5::CoreAddons

If an application links against KF5::GuiAddons but not KF5::CoreAddons,
they cannot use KImageCache.  So we use defines (set by the exported
targets) to check whether the current target has been linked against
KF5::CoreAddons, and print a warning if not (to explain the expected
kshareddatacache.h not found error).


Diffs
-

  src/CMakeLists.txt d6ee3d9987319f9a3d1f8b2f66e00d2ec4bda1e5 
  src/util/kimagecache.h 007891c757de9bedc5cf0c3b734e250004727ec4 

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


Testing
---

plasma-framework (which uses KImageCache) still builds fine.  If an older 
version of KCoreAddons (without the KCOREADDONS_LIB define) is installed, a 
warning is printed when kimagecache.h is included.  If the patch in review 
114700 is applied to KCoreAddons, the warning is not printed.


Thanks,

Alex Merry

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 114524: Load the kdoctools macro before trying to find the build deps

2013-12-28 Thread Alex Merry

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


I'm inclined to say that we don't care about the frameworks branch of kdelibs 
any more, except as a place to use git log and git blame, and so this 
should be discarded.

- Alex Merry


On Dec. 18, 2013, 3:23 a.m., Christophe Giboudeaux wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/114524/
 ---
 
 (Updated Dec. 18, 2013, 3:23 a.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Repository: kdelibs
 
 
 Description
 ---
 
 Tiny fix for kdoctools.
 
 While trying to build the split frameworks, the ones calling kdoctools were 
 failing because find_dependency was called before including 
 KF5DocToolsTargets.cmake.
 
 If find_dependency is called before, the PACKAGE_PREFIX_DIR gets changed to 
 point to the dependency install dir.
 This causes build issues if kdoctools is installed in a different prefix 
 since KDOCTOOLS_CUSTOMIZATION_DIR will have an incorrect value.
 
 
 Diffs
 -
 
   tier2/kdoctools/KF5DocToolsConfig.cmake.in 94ad675 
 
 Diff: https://git.reviewboard.kde.org/r/114524/diff/
 
 
 Testing
 ---
 
 Built and installed all the framework modules in different prefixes without 
 issue.
 
 The commit is already in the split kdoctools repo.
 
 
 Thanks,
 
 Christophe Giboudeaux
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 114524: Load the kdoctools macro before trying to find the build deps

2013-12-28 Thread Christophe Giboudeaux

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

(Updated Dec. 28, 2013, 2:02 p.m.)


Status
--

This change has been discarded.


Review request for KDE Frameworks.


Repository: kdelibs


Description
---

Tiny fix for kdoctools.

While trying to build the split frameworks, the ones calling kdoctools were 
failing because find_dependency was called before including 
KF5DocToolsTargets.cmake.

If find_dependency is called before, the PACKAGE_PREFIX_DIR gets changed to 
point to the dependency install dir.
This causes build issues if kdoctools is installed in a different prefix since 
KDOCTOOLS_CUSTOMIZATION_DIR will have an incorrect value.


Diffs
-

  tier2/kdoctools/KF5DocToolsConfig.cmake.in 94ad675 

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


Testing
---

Built and installed all the framework modules in different prefixes without 
issue.

The commit is already in the split kdoctools repo.


Thanks,

Christophe Giboudeaux

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 113805: Do not change the build types available with cmake

2013-12-28 Thread Alex Merry


 On Nov. 12, 2013, 7:24 p.m., Alexander Neundorf wrote:
  IMO the patch as it is is not good.
  
  Several things:
  1) This file, is not mandatory at all with KDE frameworks.
  You can build applications using KDE frameworks libraries without it. You 
  (the developer of the application) simply has to not-load the file 
  KDECompilerSettings.
  If the developer has decided to load this file, it is not surprising that 
  he gets modified compiler flags, because this is what he decided to do.
  
  2) You removed e.g. the flags for the build types COVERAGE and PROFILE. 
  CMake doesn't provide these build types or flags itself, so the patch 
  removes support for these buildtypes. I don't see the need to remove 
  support for profile or coverage builds. CMake itself comes with debug, 
  minsizerel, release and relwithdebinfo.
  
  3) You remove the flags for Linux and/or gcc. Why didn't you remove them 
  for other compilers/operating systems ?
  
  
  I know that what we are doing in this file is not the cmake-recommended way.
  From cmake, the variables for the flags are cache variables which are set 
  to some default. The idea is that the person who compiles some package can 
  adjust them to his liking. This is from my experience not what we expect 
  from the average KDE contributor. He should get a working set up, with 
  known compiler flags so it is easy to fix buid bugs (or avoid them in the 
  first place).
  By simply setting the normal (non-cache) variables, the person building the 
  package can set the cache variables to whatever he wants, it will be 
  overridden to what is set in KDECompilerSettings.cmake.
  Maybe the way this is done could be improved by doing something like
  if(NOT DEFINED SOME_CXX_FLAGS_ALREADY_PRESET)
 set(SOME_CXX_FLAGS_ALREADY_PRESET TRUE CACHE BOOL docs... )
 set(SOME_CXX_FLAGS --some-flag --another-flag CACHE STRING docs... 
  FORCE)
  endif()
  
  This way it would be only on the initial cmake run forced into the cache, 
  and afterwards the user should be able to change them as he wants.
  
  
 
 
 Sune Vuorela wrote:
 1) THis file is used by all kde frameworks, so it should not put in 
 surprises for the developers there. ONe shouldn't be 100% familiar with all 
 the internals to hack on stuff.
 
 2) IF we want to add such things, it should be in a 
 ECMAddProfileBuildType or similar.
 
 3) For the other compilers, the build types aren't touched.
 
 ANd note that this doesn't modify the flags that covers everything. That 
 I'm saving for another patch.
 
 And let us do thhings the cmake way, one step at a time.
 
 Alexander Neundorf wrote:
 1) I don't really understand. You say no surprises and at the same time 
 you say that developers shouldn't have to be familiar with all internals to 
 hack on stuff. If you want no surprises, remove the line 
 include(KDECompilerSettings) from the project. That's why it has been 
 separated out, to make it optional for users who don't want it. Then you get 
 plain cmake, and can/have to take care of the compiler settings yourself. Add 
 that line, and you get no need to care about internals.
 
 Is your plan also to remove the added defintions, linker flags, etc. ?
 I'm fine if you post a patch which removes the file completely. I just 
 doubt that the KDE community will be happy with this.
 
 This is the state as it was May 2006:
 
 http://quickgit.kde.org/?p=kdelibs.gita=blobhb=5713bc5ddd7f11ef73e63cf67c4a0ba69180ef3af=cmake%2Fmodules%2FFindKDE4Internal.cmake
 
 You'll see that it was quite different from what we have today. It is 
 much less than today. Back then I also started with a minimal set of flags to 
 get KDE built at all. But between then and now, all those changes have gone 
 in for a reason, each single one of them.
 
 
 P.S. I am not objecting, just giving comments.

 
 Sune Vuorela wrote:
 1) By no surprises I mean by 3rd party users, skilled in Qt and cmake, of 
 a KDE framework  - like if I end up using one at work and some of my 
 colleagues need to fix a bug or add a feature, then this would be a 
 unpleasant surprise when dealing with a standalone framework.
 
 My plan isn't - yet - to remove the file completely, but first to slice 
 it down to a size where one can see what happens and what side effects it 
 has. I have at least concrete plans for two or three more chunks to remove, 
 but I prefer slice it down chunks at a time.
 
 Alexander Neundorf wrote:
 1) Let's assume karchive. You have currently at least the following 
 options
 
 find_package(KArchive REQUIRED NO_MODULE)
 
 This finds the library, KDECompilerSettings.cmake is not involved at all.
 
 
 OR
 
 find_package(KF5 REQUIRED NO_MODULE COMPONENTS KArchive)
 
 This finds the library, via tier1/kf5umbrella/, KDECompilerSettings.cmake 
 is not involved at all.
 
 OR
 
 (when 

Re: Review Request 113805: Do not change the build types available with cmake

2013-12-28 Thread Alex Merry


 On Nov. 12, 2013, 7:24 p.m., Alexander Neundorf wrote:
  IMO the patch as it is is not good.
  
  Several things:
  1) This file, is not mandatory at all with KDE frameworks.
  You can build applications using KDE frameworks libraries without it. You 
  (the developer of the application) simply has to not-load the file 
  KDECompilerSettings.
  If the developer has decided to load this file, it is not surprising that 
  he gets modified compiler flags, because this is what he decided to do.
  
  2) You removed e.g. the flags for the build types COVERAGE and PROFILE. 
  CMake doesn't provide these build types or flags itself, so the patch 
  removes support for these buildtypes. I don't see the need to remove 
  support for profile or coverage builds. CMake itself comes with debug, 
  minsizerel, release and relwithdebinfo.
  
  3) You remove the flags for Linux and/or gcc. Why didn't you remove them 
  for other compilers/operating systems ?
  
  
  I know that what we are doing in this file is not the cmake-recommended way.
  From cmake, the variables for the flags are cache variables which are set 
  to some default. The idea is that the person who compiles some package can 
  adjust them to his liking. This is from my experience not what we expect 
  from the average KDE contributor. He should get a working set up, with 
  known compiler flags so it is easy to fix buid bugs (or avoid them in the 
  first place).
  By simply setting the normal (non-cache) variables, the person building the 
  package can set the cache variables to whatever he wants, it will be 
  overridden to what is set in KDECompilerSettings.cmake.
  Maybe the way this is done could be improved by doing something like
  if(NOT DEFINED SOME_CXX_FLAGS_ALREADY_PRESET)
 set(SOME_CXX_FLAGS_ALREADY_PRESET TRUE CACHE BOOL docs... )
 set(SOME_CXX_FLAGS --some-flag --another-flag CACHE STRING docs... 
  FORCE)
  endif()
  
  This way it would be only on the initial cmake run forced into the cache, 
  and afterwards the user should be able to change them as he wants.
  
  
 
 
 Sune Vuorela wrote:
 1) THis file is used by all kde frameworks, so it should not put in 
 surprises for the developers there. ONe shouldn't be 100% familiar with all 
 the internals to hack on stuff.
 
 2) IF we want to add such things, it should be in a 
 ECMAddProfileBuildType or similar.
 
 3) For the other compilers, the build types aren't touched.
 
 ANd note that this doesn't modify the flags that covers everything. That 
 I'm saving for another patch.
 
 And let us do thhings the cmake way, one step at a time.
 
 Alexander Neundorf wrote:
 1) I don't really understand. You say no surprises and at the same time 
 you say that developers shouldn't have to be familiar with all internals to 
 hack on stuff. If you want no surprises, remove the line 
 include(KDECompilerSettings) from the project. That's why it has been 
 separated out, to make it optional for users who don't want it. Then you get 
 plain cmake, and can/have to take care of the compiler settings yourself. Add 
 that line, and you get no need to care about internals.
 
 Is your plan also to remove the added defintions, linker flags, etc. ?
 I'm fine if you post a patch which removes the file completely. I just 
 doubt that the KDE community will be happy with this.
 
 This is the state as it was May 2006:
 
 http://quickgit.kde.org/?p=kdelibs.gita=blobhb=5713bc5ddd7f11ef73e63cf67c4a0ba69180ef3af=cmake%2Fmodules%2FFindKDE4Internal.cmake
 
 You'll see that it was quite different from what we have today. It is 
 much less than today. Back then I also started with a minimal set of flags to 
 get KDE built at all. But between then and now, all those changes have gone 
 in for a reason, each single one of them.
 
 
 P.S. I am not objecting, just giving comments.

 
 Sune Vuorela wrote:
 1) By no surprises I mean by 3rd party users, skilled in Qt and cmake, of 
 a KDE framework  - like if I end up using one at work and some of my 
 colleagues need to fix a bug or add a feature, then this would be a 
 unpleasant surprise when dealing with a standalone framework.
 
 My plan isn't - yet - to remove the file completely, but first to slice 
 it down to a size where one can see what happens and what side effects it 
 has. I have at least concrete plans for two or three more chunks to remove, 
 but I prefer slice it down chunks at a time.
 
 Alexander Neundorf wrote:
 1) Let's assume karchive. You have currently at least the following 
 options
 
 find_package(KArchive REQUIRED NO_MODULE)
 
 This finds the library, KDECompilerSettings.cmake is not involved at all.
 
 
 OR
 
 find_package(KF5 REQUIRED NO_MODULE COMPONENTS KArchive)
 
 This finds the library, via tier1/kf5umbrella/, KDECompilerSettings.cmake 
 is not involved at all.
 
 OR
 
 (when 

Review Request 114703: Improve dependency specification for kdesignerplugin

2013-12-28 Thread Alex Merry

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

Review request for KDE Frameworks.


Repository: kdesignerplugin


Description
---

Improve dependency specification

* Add a find_package for Qt5WebKitWidgets
* Make most frameworks deps optional (so it is possible to just build
  the kgendesignerplugin utility, and not the actual plugins)
* Get rid of the cruft from when it was in the kdelibs repo


NB: this doesn't need to go into TP1, but if I don't post these patches, I'll 
forget I have them.


Diffs
-

  CMakeLists.txt 38424b468d97532ffd1d19bc353c92b29a9f38b0 
  src/CMakeLists.txt 3e51888614358e71756774f6bbfd5d3c14921947 

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


Testing
---

Still builds fine.


Thanks,

Alex Merry

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Review Request 114704: Deprecate -n and -g options for kgendesignerplugin

2013-12-28 Thread Alex Merry

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

Review request for KDE Frameworks.


Repository: kdesignerplugin


Description
---

4 commits.  In reverse order:

Deprecate -n and -g options for kgendesignerplugin

The [Globals] group is a better place to set these.

Remove calls to qt5_generate_moc

kgendesignerplugin adds a #include foo.moc line, so automoc will do
the work for us.

Add a DefaultGroup Global option

This allows a default group to be set in the file, instead of the
command line.

Remove unused code in kgendesignerplugin

This code read entries in the .widgets file, but never actually used the
values it read.


NB: this doesn't need to go into TP1, but if I don't post these patches, I'll 
forget I have them.


Diffs
-

  KF5DesignerPluginMacros.cmake 43fde97f0e07a9d00d79f92bee1888bf8fcbf70c 
  README.md 5d5217e432c82149db2dcdacad5fdf2b8ee46a02 
  docs/kgendesignerplugin/man-kgendesignerplugin.1.docbook 
99d424453009f756818dc9c8f3f60558b3657453 
  src/CMakeLists.txt 3e51888614358e71756774f6bbfd5d3c14921947 
  src/kgendesignerplugin.cpp 75c35d4d3aec27afe43ada35fe497857aac8e647 

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


Testing
---

Still builds fine, including the generated plugins.


Thanks,

Alex Merry

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Review Request 114705: Remove classpreviews.{cpp, h} from kdesignerplugin

2013-12-28 Thread Alex Merry

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

Review request for KDE Frameworks.


Repository: kdesignerplugin


Description
---

Remove classpreviews.{cpp,h}

It only contains a single class - KDialogPreview - which is unused
anywhere.


NB: this doesn't need to go into TP1, but if I don't post these patches, I'll 
forget I have them.


Diffs
-

  src/CMakeLists.txt 3e51888614358e71756774f6bbfd5d3c14921947 
  src/classpreviews.h ec5545be046022917e9c6a6df6610f32965cccf9 
  src/classpreviews.cpp 1ae771162a844dd8efae3a110a1e056513ea01fe 
  src/kde.widgets 65f38391fe0cbffd1e8e000d1ce29dd483bc72fc 
  src/kdewebkit.widgets 9f07307a12b7e88f3a0ca9f5be9ad67f5640db3a 

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


Testing
---

Still builds fine


Thanks,

Alex Merry

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Review Request 114706: Fix capitalisation in KF5WebKitConfig.cmake file

2013-12-28 Thread Alex Merry

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

Review request for KDE Frameworks.


Repository: kdewebkit


Description
---

Fix capitalisation in KF5WebKitConfig.cmake file

It is WebKit, not Webkit, for both KDEWebKit and QtWebKitWidgets.


Diffs
-

  KF5WebKitConfig.cmake.in 9e3eaa8773884df95455e2b0ed0a48bb5ed64517 

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


Testing
---

kdesignerplugin with RR 114703 applied now actually builds.


Thanks,

Alex Merry

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 114703: Improve dependency specification for kdesignerplugin

2013-12-28 Thread David Faure

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

Ship it!


Looks nice.

- David Faure


On Dec. 28, 2013, 3 p.m., Alex Merry wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/114703/
 ---
 
 (Updated Dec. 28, 2013, 3 p.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Repository: kdesignerplugin
 
 
 Description
 ---
 
 Improve dependency specification
 
 * Add a find_package for Qt5WebKitWidgets
 * Make most frameworks deps optional (so it is possible to just build
   the kgendesignerplugin utility, and not the actual plugins)
 * Get rid of the cruft from when it was in the kdelibs repo
 
 
 NB: this doesn't need to go into TP1, but if I don't post these patches, I'll 
 forget I have them.
 
 
 Diffs
 -
 
   CMakeLists.txt 38424b468d97532ffd1d19bc353c92b29a9f38b0 
   src/CMakeLists.txt 3e51888614358e71756774f6bbfd5d3c14921947 
 
 Diff: https://git.reviewboard.kde.org/r/114703/diff/
 
 
 Testing
 ---
 
 Still builds fine.
 
 
 Thanks,
 
 Alex Merry
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 114703: Improve dependency specification for kdesignerplugin

2013-12-28 Thread Alex Merry

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

(Updated Dec. 28, 2013, 5:37 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks.


Repository: kdesignerplugin


Description
---

Improve dependency specification

* Add a find_package for Qt5WebKitWidgets
* Make most frameworks deps optional (so it is possible to just build
  the kgendesignerplugin utility, and not the actual plugins)
* Get rid of the cruft from when it was in the kdelibs repo


NB: this doesn't need to go into TP1, but if I don't post these patches, I'll 
forget I have them.


Diffs
-

  CMakeLists.txt 38424b468d97532ffd1d19bc353c92b29a9f38b0 
  src/CMakeLists.txt 3e51888614358e71756774f6bbfd5d3c14921947 

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


Testing
---

Still builds fine.


Thanks,

Alex Merry

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 114703: Improve dependency specification for kdesignerplugin

2013-12-28 Thread Commit Hook

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


This review has been submitted with commit 
1ff318f0d9825b7eb8c107ab31cc3fee4c53402b by Alex Merry to branch master.

- Commit Hook


On Dec. 28, 2013, 3 p.m., Alex Merry wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/114703/
 ---
 
 (Updated Dec. 28, 2013, 3 p.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Repository: kdesignerplugin
 
 
 Description
 ---
 
 Improve dependency specification
 
 * Add a find_package for Qt5WebKitWidgets
 * Make most frameworks deps optional (so it is possible to just build
   the kgendesignerplugin utility, and not the actual plugins)
 * Get rid of the cruft from when it was in the kdelibs repo
 
 
 NB: this doesn't need to go into TP1, but if I don't post these patches, I'll 
 forget I have them.
 
 
 Diffs
 -
 
   CMakeLists.txt 38424b468d97532ffd1d19bc353c92b29a9f38b0 
   src/CMakeLists.txt 3e51888614358e71756774f6bbfd5d3c14921947 
 
 Diff: https://git.reviewboard.kde.org/r/114703/diff/
 
 
 Testing
 ---
 
 Still builds fine.
 
 
 Thanks,
 
 Alex Merry
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 114704: kgendesignerplugin cleanups

2013-12-28 Thread Alex Merry

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

(Updated Dec. 28, 2013, 5:38 p.m.)


Review request for KDE Frameworks.


Changes
---

Better review title.


Summary (updated)
-

kgendesignerplugin cleanups


Repository: kdesignerplugin


Description
---

4 commits.  In reverse order:

Deprecate -n and -g options for kgendesignerplugin

The [Globals] group is a better place to set these.

Remove calls to qt5_generate_moc

kgendesignerplugin adds a #include foo.moc line, so automoc will do
the work for us.

Add a DefaultGroup Global option

This allows a default group to be set in the file, instead of the
command line.

Remove unused code in kgendesignerplugin

This code read entries in the .widgets file, but never actually used the
values it read.


NB: this doesn't need to go into TP1, but if I don't post these patches, I'll 
forget I have them.


Diffs
-

  KF5DesignerPluginMacros.cmake 43fde97f0e07a9d00d79f92bee1888bf8fcbf70c 
  README.md 5d5217e432c82149db2dcdacad5fdf2b8ee46a02 
  docs/kgendesignerplugin/man-kgendesignerplugin.1.docbook 
99d424453009f756818dc9c8f3f60558b3657453 
  src/CMakeLists.txt 3e51888614358e71756774f6bbfd5d3c14921947 
  src/kgendesignerplugin.cpp 75c35d4d3aec27afe43ada35fe497857aac8e647 

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


Testing
---

Still builds fine, including the generated plugins.


Thanks,

Alex Merry

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Review Request 114716: Separate author name from email addres in KAboutData::processCommandLine

2013-12-28 Thread David Gil Oliva

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

Review request for KDE Frameworks.


Repository: kcoreaddons


Description
---

Without this patch, the parsing of kgeography --author gives:

kgeography was written by:
   Albert Astals cidaa...@kde.org

With this patch:

kgeography was written by:
   Albert Astals Cidaa...@kde.org


Diffs
-

  src/lib/kaboutdata.cpp 3f08f25 

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


Testing
---

It builds.


Thanks,

David Gil Oliva

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 114716: Separate author name from email addres in KAboutData::processCommandLine

2013-12-28 Thread Albert Astals Cid

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



src/lib/kaboutdata.cpp
https://git.reviewboard.kde.org/r/114716/#comment33064

Not related to your patch, but i wonder if we need a \n here too?


- Albert Astals Cid


On Dec. 29, 2013, 12:55 a.m., David Gil Oliva wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/114716/
 ---
 
 (Updated Dec. 29, 2013, 12:55 a.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Repository: kcoreaddons
 
 
 Description
 ---
 
 Without this patch, the parsing of kgeography --author gives:
 
 kgeography was written by:
Albert Astals cidaa...@kde.org
 
 With this patch:
 
 kgeography was written by:
Albert Astals Cidaa...@kde.org
 
 
 Diffs
 -
 
   src/lib/kaboutdata.cpp 3f08f25 
 
 Diff: https://git.reviewboard.kde.org/r/114716/diff/
 
 
 Testing
 ---
 
 It builds.
 
 
 Thanks,
 
 David Gil Oliva
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 114716: Separate author name from email addres in KAboutData::processCommandLine

2013-12-28 Thread David Gil Oliva


 On Dec. 29, 2013, 1:05 a.m., Albert Astals Cid wrote:
  src/lib/kaboutdata.cpp, line 969
  https://git.reviewboard.kde.org/r/114716/diff/1/?file=227634#file227634line969
 
  Not related to your patch, but i wonder if we need a \n here too?

I don't think so. The output of kgeography --license gives the copyright and 
the license in a good format.


- David


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


On Dec. 29, 2013, 12:55 a.m., David Gil Oliva wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/114716/
 ---
 
 (Updated Dec. 29, 2013, 12:55 a.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Repository: kcoreaddons
 
 
 Description
 ---
 
 Without this patch, the parsing of kgeography --author gives:
 
 kgeography was written by:
Albert Astals cidaa...@kde.org
 
 With this patch:
 
 kgeography was written by:
Albert Astals Cidaa...@kde.org
 
 
 Diffs
 -
 
   src/lib/kaboutdata.cpp 3f08f25 
 
 Diff: https://git.reviewboard.kde.org/r/114716/diff/
 
 
 Testing
 ---
 
 It builds.
 
 
 Thanks,
 
 David Gil Oliva
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel