Move all links not respecting modern CMake standards to another file … #42
Conversation
onqtam#16 onqtam#16 asked to clean up the list and remove links that refer to material that does not follow modern cmake standards. I moved anything that either uses (except to say it's wrong) one of the following commands: - include_directories - add_definitions - add_compile_options Same thing if the project tries to set CMAKE_<LANG>_FLAGS. I did not remove modules repositories links unless they use the above in the README or examples.
|
|
||
| [<img src="https://rawgit.com/onqtam/awesome-cmake/master/cmake-logo.svg" align="right" width="100">](https://cmake.org/) | ||
|
|
||
| > This is an archive of pre-modern [CMake](https://cmake.org/) scripts, modules, examples and others |
There was a problem hiding this comment.
Maybe use a more concrete definition? Something like "pre-v3" for example.
It would also be nice to mention the criteria you listed in the PR description (usage of include_directories, etc.).
There was a problem hiding this comment.
pre-v3 isn't really non modern, 2.8 had some modern (ie target_*) things imo.
The commit actually has the PR description as description :)
There was a problem hiding this comment.
Oh maybe you meant in the .md file?
There was a problem hiding this comment.
Oh maybe you meant in the
.mdfile?
Yes :)
| * [cmake_format](https://github.com/cheshirekow/cmake_format) - Source code formatter for CMakeLists.txt files. [```[GPL]```][GPL] | ||
| * [cmrc](https://github.com/vector-of-bool/cmrc) - A Resource Compiler in a Single CMake Script (compile arbitrary data into a program). [```[MIT]```][MIT] |
There was a problem hiding this comment.
cmake_format and cmrc are really useful tools also for modern CMake, why they have been moved here?
There was a problem hiding this comment.
They are useful indeed, sadly they show some bad practice.
cmake_formathttps://github.com/cheshirekow/cmake_format/blame/master/cmake_format/doc/README.rst#L448- cmrc https://github.com/vector-of-bool/cmrc/blob/master/CMakeLists.txt#L13
Maybe it would be worth opening an issue there but I didn't as there were a lot of links
The cmrc one is more or less OK as it is behind an option.
There was a problem hiding this comment.
I guess it is personal taste, but excluding a formatting tool because it uses as an example of of how commands are formatted or cmrc for an use of commands inside their private testing code does not seems useful, at least IMHO.
There was a problem hiding this comment.
My issue with this is that examples is the first thing you look at, and people using formating tools usually expect them to provide good practices.
So if someone sees that they might think it's OK.
This might be a bit agressive but I've seen and fixed way too many Cmakelists.txt that had this mistake because someone saw it somewhere. As you said, it's kind of a personal matter and I would for sure prefer to keep the link. If I find some time I'll open an issue.
There was a problem hiding this comment.
Maybe the NonModernCMake.md file could include a notice explaining how resources listed there can be updated to move back to the main list? That way people could propose fixes to them.
|
Woah... Thanks for doing this! I'll get to reviewing it eventually, but it might not be in the next 1-2 weeks. |
|
LGTM - I'll just add 1 commit after merging this so that the |
…Fix #16
#16 asked to clean up the list and remove links that refer to material that does not follow modern cmake standards.
I moved anything that either uses (except to say it's wrong) one of the following commands:
include_directoriesadd_definitionsadd_compile_optionsSame thing if the project tries to set
CMAKE_<LANG>_FLAGS.I did not remove modules repositories links unless they use the above in the README or examples.