[Merge] lp:~phablet-team/gallery-app/desktop-i18n into lp:gallery-app

Olivier Tilloy olivier.tilloy at canonical.com
Tue Sep 16 11:23:43 UTC 2014


Review: Needs Fixing

With the last change I suggested, the code fails to build in an out-of-source configuration (try locally with: "mkdir build; cd build; cmake ..; make").

Looking more closely at those lines:

95	+file(RELATIVE_PATH DESKTOP_FILE_IN_IN ${CMAKE_SOURCE_DIR}
96	+     ${CMAKE_SOURCE_DIR}/desktop/${DESKTOP_FILE}.in.in)

This file command looks useless, it could be simplified to "set(DESKTOP_FILE_IN_IN desktop/${DESKTOP_FILE}.in.in)".


97	+file(RELATIVE_PATH DESKTOP_FILE_IN_IN_H ${CMAKE_SOURCE_DIR}
98	+     ${CMAKE_CURRENT_SOURCE_DIR}/${DESKTOP_FILE_IN_IN}.h)

Why is the DESKTOP_FILE_IN_IN_H path built from the current source dir? Isn’t it supposed to be generated in the current binary dir?


Please clarify, and actually test that at least an out-of-source build works (ideally an in-source build too, as I think it’s supposed to work with the current trunk) before commiting. Thanks.
-- 
https://code.launchpad.net/~phablet-team/gallery-app/desktop-i18n/+merge/234679
Your team Ubuntu Phablet Team is subscribed to branch lp:gallery-app.



More information about the Ubuntu-reviews mailing list