Re: [Review Request] Modularizing KF5
On Tuesday 01 October 2013 19:28:04 Aleix Pol wrote: Hi, Since ReviewBoard is not working for me, I decided to send this review as an e-mail. I know it's less practical, but also I think it's important to get it done and I'd rather go reasonably fast with it before we start getting too much conflicts. This patch removes all find_package usages in the root CMakeLists.txt file (not the includes, only find_package) and moves it to every different module or unsplitted folder. ENOPATCH :) ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: [Review Request] Modularizing KF5
On Wednesday 02 October 2013 10:14:12 Aurélien Gâteau wrote: On Tuesday 01 October 2013 19:28:04 Aleix Pol wrote: Hi, Since ReviewBoard is not working for me, I decided to send this review as an e-mail. I know it's less practical, but also I think it's important to get it done and I'd rather go reasonably fast with it before we start getting too much conflicts. This patch removes all find_package usages in the root CMakeLists.txt file (not the includes, only find_package) and moves it to every different module or unsplitted folder. ENOPATCH :) Ignore me. Just read the mail again, the changes are actually in the kf5_find_package branch. Aurélien ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: [Review Request] Modularizing KF5
Aurélien Gâteau wrote:
On Tuesday 01 October 2013 19:28:04 Aleix Pol wrote:
Hi,
Since ReviewBoard is not working for me, I decided to send this review as
an e-mail. I know it's less practical, but also I think it's important to
get it done and I'd rather go reasonably fast with it before we start
getting too much conflicts.
I for one a big reviewboard non-fan. I much prefer reviewing patches in
email or outside git remotes.
This patch removes all find_package usages in the root CMakeLists.txt
file (not the includes, only find_package) and moves it to every
different module or unsplitted folder.
ENOPATCH :)
He pushed a branch to the kdelibs.git repo, which is unfortunate. There's so
many dead branches in there, you'd be forgiven for not noticing. I wish it
only was allowed to contain branches which contain releases and master. All
of these 'personal work branches' should be in other remotes.
Anyway, Aleix, it looks like a lot of the use of macro_bool_to_01 is
obsolete.
All configure_file uses of HAVE_X11 seem to already use #cmakedefine01.
git grep -e HAVE_X11 --and -e cmakedefine
Replace the use of macro_bool_to_01 with set() first in a separate patch.
set(HAVE_X11 ${X11_FOUND})
Why did you comment the HAVE_QSSLSOCKET check?
Thanks,
Steve.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: [Review Request] Modularizing KF5
On Wed, Oct 2, 2013 at 10:25 AM, Stephen Kelly steve...@gmail.com wrote:
Aurélien Gâteau wrote:
On Tuesday 01 October 2013 19:28:04 Aleix Pol wrote:
Hi,
Since ReviewBoard is not working for me, I decided to send this review
as
an e-mail. I know it's less practical, but also I think it's important
to
get it done and I'd rather go reasonably fast with it before we start
getting too much conflicts.
I for one a big reviewboard non-fan. I much prefer reviewing patches in
email or outside git remotes.
This patch removes all find_package usages in the root CMakeLists.txt
file (not the includes, only find_package) and moves it to every
different module or unsplitted folder.
ENOPATCH :)
He pushed a branch to the kdelibs.git repo, which is unfortunate. There's
so
many dead branches in there, you'd be forgiven for not noticing. I wish it
only was allowed to contain branches which contain releases and master. All
of these 'personal work branches' should be in other remotes.
Anyway, Aleix, it looks like a lot of the use of macro_bool_to_01 is
obsolete.
All configure_file uses of HAVE_X11 seem to already use #cmakedefine01.
git grep -e HAVE_X11 --and -e cmakedefine
Replace the use of macro_bool_to_01 with set() first in a separate patch.
set(HAVE_X11 ${X11_FOUND})
Why did you comment the HAVE_QSSLSOCKET check?
Thanks,
Steve.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
I pushed a couple of new commits that remove the usage of macro_bool_to_01
and the HAVE_QSSLSOCKET thing.
Aleix
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: [Review Request] Modularizing KF5
Aleix Pol wrote: I pushed a couple of new commits that remove the usage of macro_bool_to_01 and the HAVE_QSSLSOCKET thing. I assume you have no intention of merging that branch? The top commit looks fine at least. Please cherry-pick it to frameworks, and rebase and clean-up the rest of the kf5_find_package branch. Thanks, Steve. ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
[Review Request] Modularizing KF5
Hi, Since ReviewBoard is not working for me, I decided to send this review as an e-mail. I know it's less practical, but also I think it's important to get it done and I'd rather go reasonably fast with it before we start getting too much conflicts. This patch removes all find_package usages in the root CMakeLists.txt file (not the includes, only find_package) and moves it to every different module or unsplitted folder. You can review the changes in the kf5_find_package branch in kdelibs. Thanks! Aleix ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
