Differential D6931

Use ecm's FindInotify.cmake
ClosedPublic
Actions

Authored by tcberner on Jul 26 2017, 12:45 PM.

Details

Reviewers
dvratil
mlaurent
Group Reviewers
FreeBSD
Commits
R94:ca2fc3a8df06: Use ecm's FindInotify.cmake

Diff Detail

Repository
R94 PIM: Message Library
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
tcberner created this revision.Jul 26 2017, 12:45 PM
Restricted Application added a project: KDE PIM. · View Herald TranscriptJul 26 2017, 12:45 PM
Restricted Application added a subscriber: KDE PIM. · View Herald Transcript
winterz added a subscriber: winterz.Jul 26 2017, 5:44 PM

lgtm. but we should wait for Laurent.

dvratil requested changes to this revision.Jul 27 2017, 9:00 AM
dvratil added inline comments.
messageviewer/src/CMakeLists.txt
24–27

Just use ${Inotify_FOUND} here directly instead of SYS_INOTIFY_H_FOUND and you can remove the line above.

This revision now requires changes to proceed.Jul 27 2017, 9:00 AM
tcberner updated this revision to Diff 17275.Jul 27 2017, 9:11 AM
tcberner edited edge metadata.

Don't define HAVE_SYS_INOTIFY_H.

dvratil requested changes to this revision.Jul 27 2017, 10:09 AM
dvratil added inline comments.
messageviewer/src/CMakeLists.txt
27

I think this is wrong. The macro_bool_to_01 evaluates the first argument and then sets the remaining arguments to 0 or 1 based on it, so it should be:

macro_bool_to_01(Inotify_FOUND HAVE_SYS_INOTIFY_H)
This revision now requires changes to proceed.Jul 27 2017, 10:09 AM
tcberner updated this revision to Diff 17280.EditedJul 27 2017, 10:16 AM
tcberner edited edge metadata.

Oh sorry.

Btw, why isn't it using #cmakedefine01? Isn't marcro_bool_to_01 deprecated?

dvratil accepted this revision.Jul 27 2017, 1:58 PM

macro_bool_to_01 is indeed deprecated, and config-messageviewer.h contains HAVE_SYS_INOTIFY_H so maybe we can replace macro_bool_to_01 just by set(HAVE_SYS_INOTIFY_H ${Inotify_FOUND}).

Feel free to adjust it if you want to, but the patch is good to go the way it is now.

This revision is now accepted and ready to land.Jul 27 2017, 1:58 PM
Closed by commit R94:ca2fc3a8df06: Use ecm's FindInotify.cmake (authored by tcberner). · Explain WhyJul 27 2017, 7:21 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmessageviewer/src/CMakeLists.txt (6 lines)

Diff 17302

View Options

messageviewer/src/CMakeLists.txt

Loading...