Differential D28076 Diff 77862 color-reload-module/CMakeLists.txt

Changeset View

Standalone View

View Options

color-reload-module/CMakeLists.txt

  • This file was added.
1​include(GNUInstallDirs)
2​
gikariUnsubmitted
Done

Would you mind setting this at the parent CMakeLists.txt?

gikari: Would you mind setting this at the parent CMakeLists.txt?
gikariUnsubmitted
Done

Why is this needed?

gikari: Why is this needed?
gikariUnsubmitted
Done

May be this could be moved to parent CMakeLists with other includes?

gikari: May be this could be moved to parent CMakeLists with other includes?
3​enable_language(C)
gikariUnsubmitted
Done

Would you mind making this as a subproject?

gikari: Would you mind making this as a subproject?
cblackAuthorUnsubmitted
Done

That's a Meson thing, not a CMake thing. CMake just has add_subdirectory, afaik.

cblack: That's a Meson thing, not a CMake thing. CMake just has add_subdirectory, afaik.
gikariUnsubmitted
Done

Why is this needed? Aren't the CMake default languages in project (parent CMakeLists) both CXX and C?

gikari: Why is this needed? Aren't the CMake default languages in project (parent CMakeLists) both…
4​
gikariUnsubmitted
Done

Move to parent CMakeLists.txt

gikari: Move to parent CMakeLists.txt
5​add_library(colorreload-gtk3-module MODULE
gikariUnsubmitted
Done

Same

gikari: Same
6​ reloader.c
7​)
gikariUnsubmitted
Done

Remove space before ( and I guess this could also go to parent CMakeLists.txt?

gikari: Remove space before `(` and I guess this could also go to parent CMakeLists.txt?
8​
9​target_link_libraries(colorreload-gtk3-module
10​ PRIVATE
gikariUnsubmitted
Done
add_library(colorreload-gtk-module SHARED
    reloader.c
)
gikari: ``` add_library(colorreload-gtk-module SHARED reloader.c ) ```
11​ PkgConfig::GLib2
gikariUnsubmitted
Done

Also, could you remove spaces around parentaces to match the CMake style with the one, that is in kded and kconf_update directories?

gikari: Also, could you remove spaces around parentaces to match the CMake style with the one, that is…
gikariUnsubmitted
Done

It seems like you use other variables, than in other parts of kde-gtk-config.
In other places it uses GTK3_LIBRARY and GLIB2_LIBRARY variables from find module. The versions also appear in target_include_directories command.

gikari: It seems like you use other variables, than in other parts of kde-gtk-config. In other places…
12​ PkgConfig::GTK+3
13​)
14​
15​set_target_properties(colorreload-gtk3-module
gikariUnsubmitted
Done

Isn't the PUBLIC/PRIVATE modifiers a good modern CMake practice? The same for target_include_directories

gikari: Isn't the PUBLIC/PRIVATE modifiers a good modern CMake practice? The same for…
gikariUnsubmitted
Done

Sorry, this is a nitpick, but could you please make this look like this:

target_link_libraries(colorreload-gtk-module
  PRIVATE
    ${GLIB_LIBRARIES}
    ${GTK_LIBRARIES}
gikari: Sorry, this is a nitpick, but could you please make this look like this: ```…
16​ PROPERTIES
17​ OUTPUT_NAME libcolorreload-gtk-module
18​)
19​
20​install(
21​ TARGETS colorreload-gtk3-module
22​ DESTINATION ${CMAKE_INSTALL_LIBDIR}/gtk-3.0/modules
23​)