[Kde-pim] Review Request: Cleaner CMakeLists.txt for KDEPIM

[Christophe Giboudeaux]

> On 2009-03-05 08:15:21, Thomas McGuire wrote:
> > trunk/KDE/kdepim/CMakeLists.txt, line 9
> > <http://reviewboard.kde.org/r/239/diff/1/?file=1887#file1887line9>
> >
> >     disabled -> disables
> >     my own typo actually :)

done.


> On 2009-03-05 08:15:21, Thomas McGuire wrote:
> > trunk/KDE/kdepim/CMakeLists.txt, line 23
> > <http://reviewboard.kde.org/r/239/diff/1/?file=1887#file1887line23>
> >
> >     Where is HAVE_CONFIG_H used?

Used in kleopatra/utils/gnupg-registry.c

It's also present in kresources/groupwise/soap/stdsoap2.h but it will never be defined for this file.


> On 2009-03-05 08:15:21, Thomas McGuire wrote:
> > trunk/KDE/kdepim/CMakeLists.txt, line 32
> > <http://reviewboard.kde.org/r/239/diff/1/?file=1887#file1887line32>
> >
> >     I guess you need to change the content of config-enterprise.h, since there is no longer a ENTERPISE_BUILD variable.

done.


> On 2009-03-05 08:15:21, Thomas McGuire wrote:
> > trunk/KDE/kdepim/CMakeLists.txt, line 134
> > <http://reviewboard.kde.org/r/239/diff/1/?file=1887#file1887line134>
> >
> >     From looking at below, KMail & KOrganizer are also disabled when qgpgme is not found, so the message should say that.

My bad. QGpgMe is still optional.


> On 2009-03-05 08:15:21, Thomas McGuire wrote:
> > trunk/KDE/kdepim/config-enterprise.h.cmake, line 1
> > <http://reviewboard.kde.org/r/239/diff/1/?file=1888#file1888line1>
> >
> >     Change to KDEPIM_ENTERPRISE_BUILD I think

done


> On 2009-03-05 08:15:21, Thomas McGuire wrote:
> > trunk/KDE/kdepim/CMakeLists.txt, line 148
> > <http://reviewboard.kde.org/r/239/diff/1/?file=1887#file1887line148>
> >
> >     Nepomuk is also used in Akonadi, so the message should be changed.

Changed: "Nepomuk extends the search and tagging functionalities in KMail and Akonadi"


> On 2009-03-05 08:15:21, Thomas McGuire wrote:
> > trunk/KDE/kdepim/CMakeLists.txt, line 244
> > <http://reviewboard.kde.org/r/239/diff/1/?file=1887#file1887line244>
> >
> >     I thjnk the whole "XYZ support" sounds strange to the user.
> >     
> >     Maybe it is better to say that KMail and KOrganizer is enabled/disabled in the normal feature log (qgpgme in this case, right?)?
> >     
> >     Or some other idea.

Removed. I added a scary message if QGpgme is not installed.


- Christophe


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


On 2009-03-05 07:21:41, Christophe Giboudeaux wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/239/
> -----------------------------------------------------------
> 
> (Updated 2009-03-05 07:21:41)
> 
> 
> Review request for KDE PIM, Allen Winter and Thomas McGuire.
> 
> 
> Summary
> -------
> 
> 
> Goal:
> Make CMakeLists.txt human readable.
> 
> Modifications:
> - Rename BUILD_EVERYTHING -> KDEPIM_BUILD_EVERYTHING
> - Rename ONLY_KLEO -> KDEPIM_ONLY_KLEO
> - Rename ENTREPRISE_BUILD -> KDEPIM_ENTREPRISE_BUILD
> - Rename KDE4_KDEPIM_NEW_DISTRLISTS -> KDEPIM_NEW_DISTRLISTS
> - Rename TOPOLOGICAL_SORT_DIR -> Boost_TOPOLOGICAL_SORT_DIR
> 
> This way, these options/items will be grouped in the CMakeCache file and in the cmake GUI app (see screenshot)
> 
> - Make QGpgme required even if KDEPIM_ONLY_KLEO is false.
> - Remove the Kode checks
> - Move the X11_Xscreensaver to KTimetracker's own CMakeLists. Since it is not a hard requirement, allow Ktimetracker to be built under Windows (*)
> - Replace add_definitions(-DFOO_SUPPORTED) with macro_bool_to_01
> - Clean as much as possible
> 
> TODO:
> - Move the libknotesresources_SRCS, libknoteseditor_SRCS, libkdgantt1_SRCS in a better place.
> - If accepted, also rename the options in kdepimlibs before commiting.
> 
> (*) Testing needed.
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdepim/CMakeLists.txt 935353 
>   trunk/KDE/kdepim/config-enterprise.h.cmake 935353 
>   trunk/KDE/kdepim/doc/CMakeLists.txt 935415 
>   trunk/KDE/kdepim/kleopatra/CMakeLists.txt 935353 
>   trunk/KDE/kdepim/kleopatra/conf/CMakeLists.txt 935353 
>   trunk/KDE/kdepim/kleopatra/conf/appearanceconfigwidget.cpp 935353 
>   trunk/KDE/kdepim/kleopatra/mainwindow.cpp 935353 
>   trunk/KDE/kdepim/kleopatra/tests/CMakeLists.txt 935353 
>   trunk/KDE/kdepim/kmail/CMakeLists.txt 935353 
>   trunk/KDE/kdepim/kmail/configuredialog.cpp 935353 
>   trunk/KDE/kdepim/kmail/configuredialog_p.h 935353 
>   trunk/KDE/kdepim/kresources/CMakeLists.txt 935353 
>   trunk/KDE/kdepim/ktimetracker/CMakeLists.txt 935353 
>   trunk/KDE/kdepim/libkleo/CMakeLists.txt 935353 
>   trunk/KDE/kdepim/libkleo/kleo/cryptobackendfactory.cpp 935353 
>   trunk/KDE/kdepim/libkleo/ui/cryptoconfigmodule.cpp 935353 
>   trunk/KDE/kdepim/libkleo/ui/messagebox.cpp 935353 
>   trunk/KDE/kdepim/libkpgp/pics/CMakeLists.txt 935353 
>   trunk/KDE/kdepim/wizards/CMakeLists.txt 935353 
> 
> Diff: http://reviewboard.kde.org/r/239/diff
> 
> 
> Testing
> -------
> 
> 
> Screenshots
> -----------
> 
> 
>   http://reviewboard.kde.org/r/239/s/49/
> 
> 
> Thanks,
> 
> Christophe
> 
>

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/