jrl-umi3218 / jrl-cmakemodules

CMake utility toolbox
https://jrl-cmakemodules.readthedocs.io/en/master/
Other
57 stars 45 forks source link

Move INSTALL_GENERATED_HEADERS option to PROJECT_ variable? #557

Open gergondet opened 1 year ago

gergondet commented 1 year ago

As I was working on #556 I noticed we provide INSTALL_GENERATED_HEADERS as an option and I think it doesn't make too much sense. On the one hand, if the headers are required for the project to work properly then disabling the option would prevent the proper use of the project. On the other hand, if the headers are not required by the project it should be up to the maintainers to decide whether or not to ship them along the project (for example config.hh can be useful outside the project even if not used internally)

What would you think of moving this option to a PROJECT_INSTALL_GENERATED_HEADERS variable instead that can be set by maintainers?

jcarpent commented 1 year ago

Fine on my side. Could you mention that the var is deprecated to smooth the transition?

nim65s commented 1 year ago

Is there any difference between INSTALL_GENERATED_HEADERS and PROJECT_INSTALL_GENERATED_HEADERS, other that the name of the variable ?

gergondet commented 1 year ago

Fine on my side. Could you mention that the var is deprecated to smooth the transition?

Sure. @wxmerkt do CMake's AUTHOR_WARNING generates warning in the ROS build farm?

Is there any difference between INSTALL_GENERATED_HEADERS and PROJECT_INSTALL_GENERATED_HEADERS, other that the name of the variable ?

INSTALL_GENERATED_HEADERS is an option but PROJECT_INSTALL_GENERATED_HEADERS would not be. We could simply move INSTALL_GENERATED_HEADERS to a variable instead of an option but I like the idea of uniformizing those PROJECT_ settings