Feature/cleanup cmake#1810
Draft
ClausKlein wants to merge 22 commits intoTencent:masterfrom
ClausKlein:feature/cleanup-cmake
Draft
Feature/cleanup cmake#1810ClausKlein wants to merge 22 commits intoTencent:masterfrom ClausKlein:feature/cleanup-cmake
ClausKlein wants to merge 22 commits intoTencent:masterfrom
ClausKlein:feature/cleanup-cmake
Conversation
cmakelint all CMakeLists.txt
to tell clang-tidy where to find the compile_commands.json file
This was referenced Dec 21, 2020
DerDakon
suggested changes
Dec 21, 2020
| endif() | ||
| elseif (CMAKE_CXX_COMPILER_ID MATCHES "Clang") | ||
| elseif(CMAKE_CXX_COMPILER_ID MATCHES "Clang") | ||
| if(NOT CMAKE_CROSSCOMPILING) |
Contributor
There was a problem hiding this comment.
This has been there before, but the check isn't necessary: CMAKE_SYSTEM_PROCESSOR is about the target system, so the check should be fine.
Author
There was a problem hiding this comment.
this CXX_FLAGS setting is not well at all!
by the way, we should not disable any warning!
This was referenced Dec 22, 2020
but which this CI zoo, prevent -Werror usages too!
no idea why this should be default off?
VERSION_LESS only usable!
Arfrever
reviewed
Dec 22, 2020
with historical cmake 3.5.1 this is not possible!
valgrind is not usable on Apple OSX
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
@miloyip why not use modern cmake?
see https://github.com/ClausKlein/modern-cmake-sample/blob/develop/README.md
and https://www.youtube.com/watch?v=rLopVhns4Zs&feature=youtu.be
this show #1812