new find module for Canberra
ClosedPublic

Authored by sitter on Feb 12 2019, 11:55 AM.

Details

Summary

used by:

  • knotification
  • (possibly also knotifyconfig at some point)
  • plasma-pa
  • kmix

Diff Detail

Repository
R240 Extra CMake Modules
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
sitter created this revision.Feb 12 2019, 11:55 AM
Restricted Application added projects: Frameworks, Build System. · View Herald TranscriptFeb 12 2019, 11:55 AM
Restricted Application added subscribers: kde-buildsystem, kde-frameworks-devel. · View Herald Transcript
sitter requested review of this revision.Feb 12 2019, 11:55 AM
apol added a subscriber: apol.Feb 12 2019, 5:43 PM
apol added inline comments.
find-modules/FindCanberra.cmake
5

Maybe you can follow the style (rst syntax?) that Volker used here D18944.

sitter updated this revision to Diff 51576.Feb 13 2019, 8:58 AM

document in rst syntax

Good point, I've also noticed that (all?) our finders now also create an IMPORTED target. Should we maybe add that too? If so I guess Canberra::Canberra would be the preferred target name?

aacid added a subscriber: aacid.Feb 16 2019, 9:57 AM

Yes, imported targets are the future/present :)

Canberra::Canberra sounds good to me as target name

Yes, imported targets are the future/present :)

Canberra::Canberra sounds good to me as target name

+1
You may also add a set_package_properties() call. see eg: D18947. This way, the users only have to use the VERSION and TYPE in their CMakeLists.txt.

find-modules/FindCanberra.cmake
10–17

The variables shall be renamed "Canberra_XXX".

If needed, the uppercase ones shall be added at the bottom of the file with a comment to indicate they only exist for compatibility.

49–50

Use the 'QUIET' keyword for both lines

54

PC_CANBERRA_LIBDIR isn't needed

59

Same thing for PC_CANBERRA_INCLUDEDIR

sitter updated this revision to Diff 51952.Feb 18 2019, 12:36 PM
  • pkgconfig is now quiet
  • variables are now camelcase
    • old variables are still set for compat
  • new imported target (also sets pkgconfig's cflags, which I presume is the sane thing to do)
  • set package description url & description
sitter marked 5 inline comments as done.Feb 18 2019, 12:36 PM
sitter updated this revision to Diff 51976.Feb 18 2019, 4:24 PM

fix bad copy paste in compat setup

cgiboudeaux accepted this revision.Feb 19 2019, 8:09 AM

Thanks! just a little thing to fix before pushing

find-modules/FindCanberra.cmake
76

Also add

FOUND_VAR Canberra_FOUND

The default for CMake 2.8.12 which ECM requires is '<UPPERCASED_NAME>_FOUND'.

This revision is now accepted and ready to land.Feb 19 2019, 8:09 AM
sitter updated this revision to Diff 52042.Feb 19 2019, 9:56 AM

explicitly set FOUND_VAR so it is camelcase too

sitter marked an inline comment as done.Feb 19 2019, 9:56 AM
This revision was automatically updated to reflect the committed changes.