Re: Review Request 110083: Make kdepim libs optional dependency for libkfbapi
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/110083/ --- (Updated Jan. 20, 2015, 4:24 p.m.) Status -- This change has been discarded. Review request for kdelibs and KDEPIM-Libraries. Repository: libkfbapi Description --- kdepim-libs are needed in the facebook library only to return user info as KABC::Addressee and note as KMime::Message, plus events use KCalCore classes. This patch makes dependency on kdepim-libs optional and disables building the event classes completely in case kdepim-libs was not found. Diffs - CMakeLists.txt 5aefdcf LibKFbAPI-KDEPIM.pc.in PRE-CREATION LibKFbAPI-KDEPIMConfig.cmake.in PRE-CREATION LibKFbAPI.pc.in af537d1 libkfbapi/CMakeLists.txt dac62bc libkfbapi/commentinfo.h e5578c7 libkfbapi/kdepim-utils.h PRE-CREATION libkfbapi/kdepim-utils.cpp PRE-CREATION libkfbapi/likeinfo.h e06052e libkfbapi/noteinfo.h 2492db1 libkfbapi/noteinfo.cpp 154593d libkfbapi/notificationinfo.h a882694 libkfbapi/userinfo.h ac22a7e libkfbapi/userinfo.cpp 26c64da libkfbapi/userinfoparser_p.h 0189a17 Diff: https://git.reviewboard.kde.org/r/110083/diff/ Testing --- Builds correctly here in both cases Thanks, Martin Klapetek
Re: Review Request 110083: Make kdepim libs optional dependency for libkfbapi
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110083/ --- (Updated May 10, 2013, 10:32 a.m.) Review request for kdelibs and KDEPIM-Libraries. Changes --- Make InvalidTimeZone const class member of UserInfo. Description --- kdepim-libs are needed in the facebook library only to return user info as KABC::Addressee and note as KMime::Message, plus events use KCalCore classes. This patch makes dependency on kdepim-libs optional and disables building the event classes completely in case kdepim-libs was not found. Diffs (updated) - CMakeLists.txt 5aefdcf LibKFbAPI-KDEPIM.pc.in PRE-CREATION LibKFbAPI-KDEPIMConfig.cmake.in PRE-CREATION LibKFbAPI.pc.in af537d1 libkfbapi/CMakeLists.txt dac62bc libkfbapi/commentinfo.h e5578c7 libkfbapi/kdepim-utils.h PRE-CREATION libkfbapi/kdepim-utils.cpp PRE-CREATION libkfbapi/likeinfo.h e06052e libkfbapi/noteinfo.h 2492db1 libkfbapi/noteinfo.cpp 154593d libkfbapi/notificationinfo.h a882694 libkfbapi/userinfo.h ac22a7e libkfbapi/userinfo.cpp 26c64da libkfbapi/userinfoparser_p.h 0189a17 Diff: http://git.reviewboard.kde.org/r/110083/diff/ Testing --- Builds correctly here in both cases Thanks, Martin Klapetek
Re: Review Request 110083: Make kdepim libs optional dependency for libkfbapi
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110083/#review32310 --- Ship it! Ship It! - Kevin Krammer On May 10, 2013, 10:32 a.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110083/ --- (Updated May 10, 2013, 10:32 a.m.) Review request for kdelibs and KDEPIM-Libraries. Description --- kdepim-libs are needed in the facebook library only to return user info as KABC::Addressee and note as KMime::Message, plus events use KCalCore classes. This patch makes dependency on kdepim-libs optional and disables building the event classes completely in case kdepim-libs was not found. Diffs - CMakeLists.txt 5aefdcf LibKFbAPI-KDEPIM.pc.in PRE-CREATION LibKFbAPI-KDEPIMConfig.cmake.in PRE-CREATION LibKFbAPI.pc.in af537d1 libkfbapi/CMakeLists.txt dac62bc libkfbapi/commentinfo.h e5578c7 libkfbapi/kdepim-utils.h PRE-CREATION libkfbapi/kdepim-utils.cpp PRE-CREATION libkfbapi/likeinfo.h e06052e libkfbapi/noteinfo.h 2492db1 libkfbapi/noteinfo.cpp 154593d libkfbapi/notificationinfo.h a882694 libkfbapi/userinfo.h ac22a7e libkfbapi/userinfo.cpp 26c64da libkfbapi/userinfoparser_p.h 0189a17 Diff: http://git.reviewboard.kde.org/r/110083/diff/ Testing --- Builds correctly here in both cases Thanks, Martin Klapetek
Re: Review Request 110083: Make kdepim libs optional dependency for libkfbapi
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110083/#review32318 --- CMakeLists.txt http://git.reviewboard.kde.org/r/110083/#comment24041 Apparently installing LibKFbAPI-KDEPIMConfig.cmake in $LIB.../cmake/LibKFbAPI makes KDEPIM-Runtime not able to find it. Is there a way or do I need to install it to $LIB.../cmake/LibKFbAPI-KDEPIM/ ? - Martin Klapetek On May 10, 2013, 10:32 a.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110083/ --- (Updated May 10, 2013, 10:32 a.m.) Review request for kdelibs and KDEPIM-Libraries. Description --- kdepim-libs are needed in the facebook library only to return user info as KABC::Addressee and note as KMime::Message, plus events use KCalCore classes. This patch makes dependency on kdepim-libs optional and disables building the event classes completely in case kdepim-libs was not found. Diffs - CMakeLists.txt 5aefdcf LibKFbAPI-KDEPIM.pc.in PRE-CREATION LibKFbAPI-KDEPIMConfig.cmake.in PRE-CREATION LibKFbAPI.pc.in af537d1 libkfbapi/CMakeLists.txt dac62bc libkfbapi/commentinfo.h e5578c7 libkfbapi/kdepim-utils.h PRE-CREATION libkfbapi/kdepim-utils.cpp PRE-CREATION libkfbapi/likeinfo.h e06052e libkfbapi/noteinfo.h 2492db1 libkfbapi/noteinfo.cpp 154593d libkfbapi/notificationinfo.h a882694 libkfbapi/userinfo.h ac22a7e libkfbapi/userinfo.cpp 26c64da libkfbapi/userinfoparser_p.h 0189a17 Diff: http://git.reviewboard.kde.org/r/110083/diff/ Testing --- Builds correctly here in both cases Thanks, Martin Klapetek
Re: Review Request 110083: Make kdepim libs optional dependency for libkfbapi
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110083/ --- (Updated May 4, 2013, 9:34 a.m.) Review request for kdelibs and KDEPIM-Libraries. Changes --- Moved invalidTimezone constant to userinfo.h, so it can be reused Description --- kdepim-libs are needed in the facebook library only to return user info as KABC::Addressee and note as KMime::Message, plus events use KCalCore classes. This patch makes dependency on kdepim-libs optional and disables building the event classes completely in case kdepim-libs was not found. Diffs (updated) - LibKFbAPI-KDEPIMConfig.cmake.in PRE-CREATION CMakeLists.txt 5aefdcf LibKFbAPI-KDEPIM.pc.in PRE-CREATION LibKFbAPI.pc.in af537d1 libkfbapi/CMakeLists.txt dac62bc libkfbapi/commentinfo.h e5578c7 libkfbapi/kdepim-utils.h PRE-CREATION libkfbapi/kdepim-utils.cpp PRE-CREATION libkfbapi/likeinfo.h e06052e libkfbapi/noteinfo.h 2492db1 libkfbapi/noteinfo.cpp 154593d libkfbapi/notificationinfo.h a882694 libkfbapi/userinfo.h ac22a7e libkfbapi/userinfo.cpp 26c64da libkfbapi/userinfoparser_p.h 0189a17 Diff: http://git.reviewboard.kde.org/r/110083/diff/ Testing --- Builds correctly here in both cases Thanks, Martin Klapetek
Re: Review Request 110083: Make kdepim libs optional dependency for libkfbapi
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110083/#review32015 --- libkfbapi/userinfo.h http://git.reviewboard.kde.org/r/110083/#comment23854 No, this would be global symbol. I meant as a constant of UserInfo, i.e. UserInfo::InvalidTimeZone. It is a special value returned by UserInfo::timezone(), right? - Kevin Krammer On May 4, 2013, 9:34 a.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110083/ --- (Updated May 4, 2013, 9:34 a.m.) Review request for kdelibs and KDEPIM-Libraries. Description --- kdepim-libs are needed in the facebook library only to return user info as KABC::Addressee and note as KMime::Message, plus events use KCalCore classes. This patch makes dependency on kdepim-libs optional and disables building the event classes completely in case kdepim-libs was not found. Diffs - LibKFbAPI-KDEPIMConfig.cmake.in PRE-CREATION CMakeLists.txt 5aefdcf LibKFbAPI-KDEPIM.pc.in PRE-CREATION LibKFbAPI.pc.in af537d1 libkfbapi/CMakeLists.txt dac62bc libkfbapi/commentinfo.h e5578c7 libkfbapi/kdepim-utils.h PRE-CREATION libkfbapi/kdepim-utils.cpp PRE-CREATION libkfbapi/likeinfo.h e06052e libkfbapi/noteinfo.h 2492db1 libkfbapi/noteinfo.cpp 154593d libkfbapi/notificationinfo.h a882694 libkfbapi/userinfo.h ac22a7e libkfbapi/userinfo.cpp 26c64da libkfbapi/userinfoparser_p.h 0189a17 Diff: http://git.reviewboard.kde.org/r/110083/diff/ Testing --- Builds correctly here in both cases Thanks, Martin Klapetek
Re: Review Request 110083: Make kdepim libs optional dependency for libkfbapi
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110083/#review31926 --- CMakeLists.txt http://git.reviewboard.kde.org/r/110083/#comment23805 I would change that to WITH_KDEPIMLIBS, i.e. without underscore between KDEPIM and LIBS. For me KDEPIM_LIBS suggests libraries from module kdepim, while this is about libraries from kdepimlibs libkfbapi/kdepim-utils.h http://git.reviewboard.kde.org/r/110083/#comment23806 I think you could forward declare Addressee here libkfbapi/kdepim-utils.h http://git.reviewboard.kde.org/r/110083/#comment23807 those two most likely as well libkfbapi/kdepim-utils.cpp http://git.reviewboard.kde.org/r/110083/#comment23808 how do you arrive at this value? is this documented somewhere? - Kevin Krammer On April 29, 2013, 11:18 a.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110083/ --- (Updated April 29, 2013, 11:18 a.m.) Review request for kdelibs and KDEPIM-Libraries. Description --- kdepim-libs are needed in the facebook library only to return user info as KABC::Addressee and note as KMime::Message, plus events use KCalCore classes. This patch makes dependency on kdepim-libs optional and disables building the event classes completely in case kdepim-libs was not found. Diffs - CMakeLists.txt 5aefdcf LibKFbAPI-KDEPIM.pc.in PRE-CREATION LibKFbAPI-KDEPIMConfig.cmake.in PRE-CREATION LibKFbAPI.pc.in af537d1 libkfbapi/CMakeLists.txt dac62bc libkfbapi/commentinfo.h e5578c7 libkfbapi/kdepim-utils.h PRE-CREATION libkfbapi/kdepim-utils.cpp PRE-CREATION libkfbapi/likeinfo.h e06052e libkfbapi/noteinfo.h 2492db1 libkfbapi/noteinfo.cpp 154593d libkfbapi/notificationinfo.h a882694 libkfbapi/userinfo.h ac22a7e libkfbapi/userinfo.cpp 26c64da libkfbapi/userinfoparser_p.h 0189a17 Diff: http://git.reviewboard.kde.org/r/110083/diff/ Testing --- Builds correctly here in both cases Thanks, Martin Klapetek
Re: Review Request 110083: Make kdepim libs optional dependency for libkfbapi
On May 3, 2013, 6:43 a.m., Kevin Krammer wrote: libkfbapi/kdepim-utils.cpp, line 4 http://git.reviewboard.kde.org/r/110083/diff/4/?file=141390#file141390line4 how do you arrive at this value? is this documented somewhere? 42 is the universal answer for everything! Besides that, this is from the original code by Thomas; there are only 26 timezones, so I think 42 is ok, but I'll add a comment On May 3, 2013, 6:43 a.m., Kevin Krammer wrote: CMakeLists.txt, line 18 http://git.reviewboard.kde.org/r/110083/diff/4/?file=141383#file141383line18 I would change that to WITH_KDEPIMLIBS, i.e. without underscore between KDEPIM and LIBS. For me KDEPIM_LIBS suggests libraries from module kdepim, while this is about libraries from kdepimlibs Makes sense, I'll change it - Martin --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110083/#review31926 --- On April 29, 2013, 11:18 a.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110083/ --- (Updated April 29, 2013, 11:18 a.m.) Review request for kdelibs and KDEPIM-Libraries. Description --- kdepim-libs are needed in the facebook library only to return user info as KABC::Addressee and note as KMime::Message, plus events use KCalCore classes. This patch makes dependency on kdepim-libs optional and disables building the event classes completely in case kdepim-libs was not found. Diffs - CMakeLists.txt 5aefdcf LibKFbAPI-KDEPIM.pc.in PRE-CREATION LibKFbAPI-KDEPIMConfig.cmake.in PRE-CREATION LibKFbAPI.pc.in af537d1 libkfbapi/CMakeLists.txt dac62bc libkfbapi/commentinfo.h e5578c7 libkfbapi/kdepim-utils.h PRE-CREATION libkfbapi/kdepim-utils.cpp PRE-CREATION libkfbapi/likeinfo.h e06052e libkfbapi/noteinfo.h 2492db1 libkfbapi/noteinfo.cpp 154593d libkfbapi/notificationinfo.h a882694 libkfbapi/userinfo.h ac22a7e libkfbapi/userinfo.cpp 26c64da libkfbapi/userinfoparser_p.h 0189a17 Diff: http://git.reviewboard.kde.org/r/110083/diff/ Testing --- Builds correctly here in both cases Thanks, Martin Klapetek
Re: Review Request 110083: Make kdepim libs optional dependency for libkfbapi
On May 2, 2013, 8:49 a.m., Christophe Giboudeaux wrote:
LibKFbAPI-KDEPIMConfig.cmake.in, line 24
http://git.reviewboard.kde.org/r/110083/diff/4/?file=141385#file141385line24
CMake will stop at this point.
Exported targets can only be loaded once.
LibKFbAPITargetsWithPrefix.cmake is already loaded by
LibKFbAPIConfig.cmake
One solution is to change this line: install(TARGETS kfbapi
kfbapikdepim EXPORT kfbapiLibraryTargets ${INSTALL_TARGETS_DEFAULT_ARGS})
to use a different export name for kfbapikdepim (and then install a
different export file for this library) but then you also have to require a
different CMake version (multiple export-sets in a given project is a
2.8.10 feature iirc)
Christophe Giboudeaux wrote:
hm, forget this, I wasn't fully awake and sent the wrong comment.
Ok, thanks for looking anyway!
- Martin
---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/110083/#review31872
---
On April 29, 2013, 11:18 a.m., Martin Klapetek wrote:
---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/110083/
---
(Updated April 29, 2013, 11:18 a.m.)
Review request for kdelibs and KDEPIM-Libraries.
Description
---
kdepim-libs are needed in the facebook library only to return user info as
KABC::Addressee and note as KMime::Message, plus events use KCalCore classes.
This patch makes dependency on kdepim-libs optional and disables building the
event classes completely in case kdepim-libs was not found.
Diffs
-
CMakeLists.txt 5aefdcf
LibKFbAPI-KDEPIM.pc.in PRE-CREATION
LibKFbAPI-KDEPIMConfig.cmake.in PRE-CREATION
LibKFbAPI.pc.in af537d1
libkfbapi/CMakeLists.txt dac62bc
libkfbapi/commentinfo.h e5578c7
libkfbapi/kdepim-utils.h PRE-CREATION
libkfbapi/kdepim-utils.cpp PRE-CREATION
libkfbapi/likeinfo.h e06052e
libkfbapi/noteinfo.h 2492db1
libkfbapi/noteinfo.cpp 154593d
libkfbapi/notificationinfo.h a882694
libkfbapi/userinfo.h ac22a7e
libkfbapi/userinfo.cpp 26c64da
libkfbapi/userinfoparser_p.h 0189a17
Diff: http://git.reviewboard.kde.org/r/110083/diff/
Testing
---
Builds correctly here in both cases
Thanks,
Martin Klapetek
Re: Review Request 110083: Make kdepim libs optional dependency for libkfbapi
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110083/ --- (Updated May 3, 2013, 9:41 a.m.) Review request for kdelibs and KDEPIM-Libraries. Changes --- * Change WITH_KDEPIM_LIBS to WITH_KDEPIMLIBS * Forward declare classes * Add comment for invalid timezone constant Description --- kdepim-libs are needed in the facebook library only to return user info as KABC::Addressee and note as KMime::Message, plus events use KCalCore classes. This patch makes dependency on kdepim-libs optional and disables building the event classes completely in case kdepim-libs was not found. Diffs (updated) - CMakeLists.txt 5aefdcf LibKFbAPI-KDEPIM.pc.in PRE-CREATION LibKFbAPI-KDEPIMConfig.cmake.in PRE-CREATION LibKFbAPI.pc.in af537d1 libkfbapi/CMakeLists.txt dac62bc libkfbapi/commentinfo.h e5578c7 libkfbapi/kdepim-utils.h PRE-CREATION libkfbapi/kdepim-utils.cpp PRE-CREATION libkfbapi/likeinfo.h e06052e libkfbapi/noteinfo.h 2492db1 libkfbapi/noteinfo.cpp 154593d libkfbapi/notificationinfo.h a882694 libkfbapi/userinfo.h ac22a7e libkfbapi/userinfo.cpp 26c64da libkfbapi/userinfoparser_p.h 0189a17 Diff: http://git.reviewboard.kde.org/r/110083/diff/ Testing --- Builds correctly here in both cases Thanks, Martin Klapetek
Re: Review Request 110083: Make kdepim libs optional dependency for libkfbapi
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110083/#review31934 --- libkfbapi/kdepim-utils.cpp http://git.reviewboard.kde.org/r/110083/#comment23813 Hmm. I see the same value being defined in userinfo.cpp so maybe it should be a public const int or enum value there and reused here? Also: what about using a negative value? - Kevin Krammer On May 3, 2013, 9:41 a.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110083/ --- (Updated May 3, 2013, 9:41 a.m.) Review request for kdelibs and KDEPIM-Libraries. Description --- kdepim-libs are needed in the facebook library only to return user info as KABC::Addressee and note as KMime::Message, plus events use KCalCore classes. This patch makes dependency on kdepim-libs optional and disables building the event classes completely in case kdepim-libs was not found. Diffs - CMakeLists.txt 5aefdcf LibKFbAPI-KDEPIM.pc.in PRE-CREATION LibKFbAPI-KDEPIMConfig.cmake.in PRE-CREATION LibKFbAPI.pc.in af537d1 libkfbapi/CMakeLists.txt dac62bc libkfbapi/commentinfo.h e5578c7 libkfbapi/kdepim-utils.h PRE-CREATION libkfbapi/kdepim-utils.cpp PRE-CREATION libkfbapi/likeinfo.h e06052e libkfbapi/noteinfo.h 2492db1 libkfbapi/noteinfo.cpp 154593d libkfbapi/notificationinfo.h a882694 libkfbapi/userinfo.h ac22a7e libkfbapi/userinfo.cpp 26c64da libkfbapi/userinfoparser_p.h 0189a17 Diff: http://git.reviewboard.kde.org/r/110083/diff/ Testing --- Builds correctly here in both cases Thanks, Martin Klapetek
Re: Review Request 110083: Make kdepim libs optional dependency for libkfbapi
On May 3, 2013, 10:35 a.m., Kevin Krammer wrote: libkfbapi/kdepim-utils.cpp, line 30 http://git.reviewboard.kde.org/r/110083/diff/5/?file=141850#file141850line30 Hmm. I see the same value being defined in userinfo.cpp so maybe it should be a public const int or enum value there and reused here? Also: what about using a negative value? Sure, I can make it reused. But it can't be negative as it's the offset from UTC, so negative numbers are valid too. - Martin --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110083/#review31934 --- On May 3, 2013, 9:41 a.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110083/ --- (Updated May 3, 2013, 9:41 a.m.) Review request for kdelibs and KDEPIM-Libraries. Description --- kdepim-libs are needed in the facebook library only to return user info as KABC::Addressee and note as KMime::Message, plus events use KCalCore classes. This patch makes dependency on kdepim-libs optional and disables building the event classes completely in case kdepim-libs was not found. Diffs - CMakeLists.txt 5aefdcf LibKFbAPI-KDEPIM.pc.in PRE-CREATION LibKFbAPI-KDEPIMConfig.cmake.in PRE-CREATION LibKFbAPI.pc.in af537d1 libkfbapi/CMakeLists.txt dac62bc libkfbapi/commentinfo.h e5578c7 libkfbapi/kdepim-utils.h PRE-CREATION libkfbapi/kdepim-utils.cpp PRE-CREATION libkfbapi/likeinfo.h e06052e libkfbapi/noteinfo.h 2492db1 libkfbapi/noteinfo.cpp 154593d libkfbapi/notificationinfo.h a882694 libkfbapi/userinfo.h ac22a7e libkfbapi/userinfo.cpp 26c64da libkfbapi/userinfoparser_p.h 0189a17 Diff: http://git.reviewboard.kde.org/r/110083/diff/ Testing --- Builds correctly here in both cases Thanks, Martin Klapetek
Re: Review Request 110083: Make kdepim libs optional dependency for libkfbapi
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110083/#review31870 --- Ping? - Martin Klapetek On April 29, 2013, 11:18 a.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110083/ --- (Updated April 29, 2013, 11:18 a.m.) Review request for kdelibs and KDEPIM-Libraries. Description --- kdepim-libs are needed in the facebook library only to return user info as KABC::Addressee and note as KMime::Message, plus events use KCalCore classes. This patch makes dependency on kdepim-libs optional and disables building the event classes completely in case kdepim-libs was not found. Diffs - CMakeLists.txt 5aefdcf LibKFbAPI-KDEPIM.pc.in PRE-CREATION LibKFbAPI-KDEPIMConfig.cmake.in PRE-CREATION LibKFbAPI.pc.in af537d1 libkfbapi/CMakeLists.txt dac62bc libkfbapi/commentinfo.h e5578c7 libkfbapi/kdepim-utils.h PRE-CREATION libkfbapi/kdepim-utils.cpp PRE-CREATION libkfbapi/likeinfo.h e06052e libkfbapi/noteinfo.h 2492db1 libkfbapi/noteinfo.cpp 154593d libkfbapi/notificationinfo.h a882694 libkfbapi/userinfo.h ac22a7e libkfbapi/userinfo.cpp 26c64da libkfbapi/userinfoparser_p.h 0189a17 Diff: http://git.reviewboard.kde.org/r/110083/diff/ Testing --- Builds correctly here in both cases Thanks, Martin Klapetek
Re: Review Request 110083: Make kdepim libs optional dependency for libkfbapi
---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/110083/#review31872
---
LibKFbAPI-KDEPIMConfig.cmake.in
http://git.reviewboard.kde.org/r/110083/#comment23766
CMake will stop at this point.
Exported targets can only be loaded once.
LibKFbAPITargetsWithPrefix.cmake is already loaded by LibKFbAPIConfig.cmake
One solution is to change this line: install(TARGETS kfbapi kfbapikdepim
EXPORT kfbapiLibraryTargets ${INSTALL_TARGETS_DEFAULT_ARGS}) to use a different
export name for kfbapikdepim (and then install a different export file for this
library) but then you also have to require a different CMake version (multiple
export-sets in a given project is a 2.8.10 feature iirc)
- Christophe Giboudeaux
On April 29, 2013, 11:18 a.m., Martin Klapetek wrote:
---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/110083/
---
(Updated April 29, 2013, 11:18 a.m.)
Review request for kdelibs and KDEPIM-Libraries.
Description
---
kdepim-libs are needed in the facebook library only to return user info as
KABC::Addressee and note as KMime::Message, plus events use KCalCore classes.
This patch makes dependency on kdepim-libs optional and disables building the
event classes completely in case kdepim-libs was not found.
Diffs
-
CMakeLists.txt 5aefdcf
LibKFbAPI-KDEPIM.pc.in PRE-CREATION
LibKFbAPI-KDEPIMConfig.cmake.in PRE-CREATION
LibKFbAPI.pc.in af537d1
libkfbapi/CMakeLists.txt dac62bc
libkfbapi/commentinfo.h e5578c7
libkfbapi/kdepim-utils.h PRE-CREATION
libkfbapi/kdepim-utils.cpp PRE-CREATION
libkfbapi/likeinfo.h e06052e
libkfbapi/noteinfo.h 2492db1
libkfbapi/noteinfo.cpp 154593d
libkfbapi/notificationinfo.h a882694
libkfbapi/userinfo.h ac22a7e
libkfbapi/userinfo.cpp 26c64da
libkfbapi/userinfoparser_p.h 0189a17
Diff: http://git.reviewboard.kde.org/r/110083/diff/
Testing
---
Builds correctly here in both cases
Thanks,
Martin Klapetek
Re: Review Request 110083: Make kdepim libs optional dependency for libkfbapi
On May 2, 2013, 8:49 a.m., Christophe Giboudeaux wrote:
LibKFbAPI-KDEPIMConfig.cmake.in, line 24
http://git.reviewboard.kde.org/r/110083/diff/4/?file=141385#file141385line24
CMake will stop at this point.
Exported targets can only be loaded once.
LibKFbAPITargetsWithPrefix.cmake is already loaded by
LibKFbAPIConfig.cmake
One solution is to change this line: install(TARGETS kfbapi
kfbapikdepim EXPORT kfbapiLibraryTargets ${INSTALL_TARGETS_DEFAULT_ARGS})
to use a different export name for kfbapikdepim (and then install a
different export file for this library) but then you also have to require a
different CMake version (multiple export-sets in a given project is a
2.8.10 feature iirc)
hm, forget this, I wasn't fully awake and sent the wrong comment.
- Christophe
---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/110083/#review31872
---
On April 29, 2013, 11:18 a.m., Martin Klapetek wrote:
---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/110083/
---
(Updated April 29, 2013, 11:18 a.m.)
Review request for kdelibs and KDEPIM-Libraries.
Description
---
kdepim-libs are needed in the facebook library only to return user info as
KABC::Addressee and note as KMime::Message, plus events use KCalCore classes.
This patch makes dependency on kdepim-libs optional and disables building the
event classes completely in case kdepim-libs was not found.
Diffs
-
CMakeLists.txt 5aefdcf
LibKFbAPI-KDEPIM.pc.in PRE-CREATION
LibKFbAPI-KDEPIMConfig.cmake.in PRE-CREATION
LibKFbAPI.pc.in af537d1
libkfbapi/CMakeLists.txt dac62bc
libkfbapi/commentinfo.h e5578c7
libkfbapi/kdepim-utils.h PRE-CREATION
libkfbapi/kdepim-utils.cpp PRE-CREATION
libkfbapi/likeinfo.h e06052e
libkfbapi/noteinfo.h 2492db1
libkfbapi/noteinfo.cpp 154593d
libkfbapi/notificationinfo.h a882694
libkfbapi/userinfo.h ac22a7e
libkfbapi/userinfo.cpp 26c64da
libkfbapi/userinfoparser_p.h 0189a17
Diff: http://git.reviewboard.kde.org/r/110083/diff/
Testing
---
Builds correctly here in both cases
Thanks,
Martin Klapetek
Re: Review Request 110083: Make kdepim libs optional dependency for libkfbapi
On April 26, 2013, 8:09 a.m., Kevin Krammer wrote: I am wondering if it wouldnt't make more sense to have the PIM integration as a separate lib, so that libkfbapi has a stable API and ABI at all times. I like that idea. I'll update the review with it then :) - Martin --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110083/#review31608 --- On April 26, 2013, 7:37 a.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110083/ --- (Updated April 26, 2013, 7:37 a.m.) Review request for kdelibs and KDEPIM-Libraries. Description --- kdepim-libs are needed in the facebook library only to return user info as KABC::Addressee and note as KMime::Message, plus events use KCalCore classes. This patch makes dependency on kdepim-libs optional and disables building the event classes completely in case kdepim-libs was not found. Diffs - CMakeLists.txt 5aefdcf libkfbapi/CMakeLists.txt dac62bc libkfbapi/commentinfo.h e5578c7 libkfbapi/config.h.cmake PRE-CREATION libkfbapi/likeinfo.h e06052e libkfbapi/noteinfo.h 2492db1 libkfbapi/noteinfo.cpp 154593d libkfbapi/notificationinfo.h a882694 libkfbapi/userinfo.h ac22a7e libkfbapi/userinfo.cpp 26c64da libkfbapi/userinfoparser_p.h 0189a17 Diff: http://git.reviewboard.kde.org/r/110083/diff/ Testing --- Builds correctly here in both cases Thanks, Martin Klapetek
Re: Review Request 110083: Make kdepim libs optional dependency for libkfbapi
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110083/ --- (Updated April 29, 2013, 11:17 a.m.) Review request for kdelibs and KDEPIM-Libraries. Changes --- Update libkfbapi to have the kdepim interface as optional additional library Description --- kdepim-libs are needed in the facebook library only to return user info as KABC::Addressee and note as KMime::Message, plus events use KCalCore classes. This patch makes dependency on kdepim-libs optional and disables building the event classes completely in case kdepim-libs was not found. Diffs (updated) - CMakeLists.txt 5aefdcf libkfbapi/CMakeLists.txt dac62bc libkfbapi/commentinfo.h e5578c7 libkfbapi/kdepim-utils.h PRE-CREATION libkfbapi/kdepim-utils.cpp PRE-CREATION libkfbapi/likeinfo.h e06052e libkfbapi/noteinfo.h 2492db1 libkfbapi/noteinfo.cpp 154593d libkfbapi/notificationinfo.h a882694 libkfbapi/userinfo.h ac22a7e libkfbapi/userinfo.cpp 26c64da libkfbapi/userinfoparser_p.h 0189a17 Diff: http://git.reviewboard.kde.org/r/110083/diff/ Testing --- Builds correctly here in both cases Thanks, Martin Klapetek
Re: Review Request 110083: Make kdepim libs optional dependency for libkfbapi
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110083/ --- (Updated April 29, 2013, 11:18 a.m.) Review request for kdelibs and KDEPIM-Libraries. Changes --- Upload correct patch :) Description --- kdepim-libs are needed in the facebook library only to return user info as KABC::Addressee and note as KMime::Message, plus events use KCalCore classes. This patch makes dependency on kdepim-libs optional and disables building the event classes completely in case kdepim-libs was not found. Diffs (updated) - CMakeLists.txt 5aefdcf LibKFbAPI-KDEPIM.pc.in PRE-CREATION LibKFbAPI-KDEPIMConfig.cmake.in PRE-CREATION LibKFbAPI.pc.in af537d1 libkfbapi/CMakeLists.txt dac62bc libkfbapi/commentinfo.h e5578c7 libkfbapi/kdepim-utils.h PRE-CREATION libkfbapi/kdepim-utils.cpp PRE-CREATION libkfbapi/likeinfo.h e06052e libkfbapi/noteinfo.h 2492db1 libkfbapi/noteinfo.cpp 154593d libkfbapi/notificationinfo.h a882694 libkfbapi/userinfo.h ac22a7e libkfbapi/userinfo.cpp 26c64da libkfbapi/userinfoparser_p.h 0189a17 Diff: http://git.reviewboard.kde.org/r/110083/diff/ Testing --- Builds correctly here in both cases Thanks, Martin Klapetek
Re: Review Request 110083: Make kdepim libs optional dependency for libkfbapi
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110083/ --- (Updated April 26, 2013, 7:37 a.m.) Review request for kdelibs and KDEPIM-Libraries. Changes --- As this is my first patch making a lib build stuff conditionally, I'd like some opinion on this. Thanks Description --- kdepim-libs are needed in the facebook library only to return user info as KABC::Addressee and note as KMime::Message, plus events use KCalCore classes. This patch makes dependency on kdepim-libs optional and disables building the event classes completely in case kdepim-libs was not found. Diffs - CMakeLists.txt 5aefdcf libkfbapi/CMakeLists.txt dac62bc libkfbapi/commentinfo.h e5578c7 libkfbapi/config.h.cmake PRE-CREATION libkfbapi/likeinfo.h e06052e libkfbapi/noteinfo.h 2492db1 libkfbapi/noteinfo.cpp 154593d libkfbapi/notificationinfo.h a882694 libkfbapi/userinfo.h ac22a7e libkfbapi/userinfo.cpp 26c64da libkfbapi/userinfoparser_p.h 0189a17 Diff: http://git.reviewboard.kde.org/r/110083/diff/ Testing --- Builds correctly here in both cases Thanks, Martin Klapetek
Re: Review Request 110083: Make kdepim libs optional dependency for libkfbapi
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110083/#review31608 --- I am wondering if it wouldnt't make more sense to have the PIM integration as a separate lib, so that libkfbapi has a stable API and ABI at all times. - Kevin Krammer On April 26, 2013, 7:37 a.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110083/ --- (Updated April 26, 2013, 7:37 a.m.) Review request for kdelibs and KDEPIM-Libraries. Description --- kdepim-libs are needed in the facebook library only to return user info as KABC::Addressee and note as KMime::Message, plus events use KCalCore classes. This patch makes dependency on kdepim-libs optional and disables building the event classes completely in case kdepim-libs was not found. Diffs - CMakeLists.txt 5aefdcf libkfbapi/CMakeLists.txt dac62bc libkfbapi/commentinfo.h e5578c7 libkfbapi/config.h.cmake PRE-CREATION libkfbapi/likeinfo.h e06052e libkfbapi/noteinfo.h 2492db1 libkfbapi/noteinfo.cpp 154593d libkfbapi/notificationinfo.h a882694 libkfbapi/userinfo.h ac22a7e libkfbapi/userinfo.cpp 26c64da libkfbapi/userinfoparser_p.h 0189a17 Diff: http://git.reviewboard.kde.org/r/110083/diff/ Testing --- Builds correctly here in both cases Thanks, Martin Klapetek
