Code review comment for lp:~njpatel/unity/nicer-glib-signals

Revision history for this message
Sam Spilsbury (smspillaz) wrote :

Yo Dawg,

I went a little deeper into this code and found that in tests/ you've added what looks like generated marshallers for glib callbacks. It might be better to autogenerate that a build time instead of shipping it in the source distribution since the *.list could change whenever we need a new marshaller. For example

add_executable (test-signals ... ${CMAKE_CURRENT_BINARY_DIR}/generated/test_glib_signals_utils_marshal.cpp
...)

file (MAKE_DIRECTORY ${CMAKE_CURRENT_BUILD_DIR}/generated)
add_custom_command (TARGET test-signals PRE_BUILD
COMMAND glib-genmarshals ${CMAKE_CURRENT_SOURCE_DIR}/test_glib_signals_utils_marshal.list
WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/generated)

That way we can create and perceive the marshallers at the same time, without the person doing the compiling ever knowing that!

(I can't comment on the nature of the code itself, knowing little about glib, however I do intend to run the tests and try it out)

« Back to merge proposal