Podcast #128: We chat with Kent C Dodds about why he loves React and discuss what life was like in the dark days before Git. Listen now.

Hot answers tagged

8

Since you have asked for CMake best practices let me concentrate on that part. Here are my recommendations: Keep it cross-platform. Avoid compiler/platform specific settings. Reorder your project to use one sub-folder with one CMakeLists.txt for each target. Don't use ExternalProject_Add() if your project is on Git. Use Git Submodules instead. For external ...


6

Disclaimer: I'm not a CMake user, so this review may be shorter, and mostly focused on style and readability. This questions needs some love though, so I'll do my best. Why do you have comments in the code below? find_package(FLAC) # test if FLAC is installed on the system if(${FLAC_FOUND}) # do something if it is found, maybe tell the user else(${...


5

Good call using 64-bit ints for output. Don't use eight bit ints for inputs. I'd use a plain int without specifying length. Note that a 32-bit int value to the second power will fit into 64 bits, and zero or one to the 0x7fffffff (0xffffffff for unsigned) power will also. @200_success mentioned eliminating duplicate power3(base, power/2). That's a great ...


4

As commented by @drescherjm and @NicolasHolthaus your example code already looks pretty good. If you are getting less verbose, you will always loose some information. Minimal Version To demonstrate this, let's take a look at a minimal version of your code: cmake_minimum_required(VERSION 3.4) project(AwesomeScheduler) set(CMAKE_CXX_STANDARD 14) set(...


4

You don't want to define objects in the .h file. Two .c files independently including constants.h would result in multiple definition error at link time. So, a constants.h should declare the variables as extern, and another autogenerated constants.c file shall define them. The approach makes portability at the binary level questionable. The generated ...


3

Here are some ideas about how to reduce the complexity and thereby increase the readability of your module's code: You could merge prefer_static_libs(), find_package() and restore_preferred_libs() into a single dedicated macro like macro(rs_find_package) set(_OLD_FIND_SUFFIXES ${CMAKE_FIND_LIBRARY_SUFFIXES}) set(CMAKE_FIND_LIBRARY_SUFFIXES ".lib" "....


3

This generally looks pretty good, but I have a few suggestions that might allow you to make your code and build system a little better. Replace inc with src The source code for your projects is apparently going into inc. I would expect source code in src and header-only includes perhaps in inc. I'd suggest changing from inc to src to make it clear that ...


3

You could DRY this up: { ++accumulator; return power3(base, power/2) * power3(base, power/2); } else { ++accumulator; return base * power3(base, power/2) * power3(base, power/2); } Bit too much repeated code for my taste. Put some in a function with a "Name" Why is '\n' special? // absorb newline character (if existant) from previous ...


2

The documentation for CMake urges you not to use glob to detect every source file. The reasoning behind it is this: if you add a file it will not detect it. You will have to rerun CMake manually every time you add or remove source files. If that is acceptable to you, then it is okay to use globbing, just be advised that it is generally not considered a good ...


2

Your implementation of power3() is inefficient, as it contains a common subexpression power3(base, power/2) that is costly to compute.


2

I see some things that may help you improve your code. Understand the risk of undocumented directives Although they have been in the code for years, both of these directives are undocumented: set(CMAKE_DISABLE_IN_SOURCE_BUILD ON) set(CMAKE_DISABLE_SOURCE_CHANGES ON) That means that they could disappear without warning in future versions of CMake. Their ...


2

Overall I think you have all the major pieces there, but I would also taking your find module a step further and create a target if QScintilla is found. You can do this by adding the following: if(QSCINTILLA_LIBRARY AND QSCINTILLA_INCLUDE_DIR) set(QSCINTILLA_LIBRARIES ${QSCINTILLA_LIBRARY}) set(QSCINTILLA_INCLUDE_DIRS ${QSCINTILLA_INCLUDE_DIR}) if(...


2

I'm by no means a professional CMake user, I try to follow best practices though and have seen many talks and articles about modern CMake best practices, so lets go through your CMakeLists. set(CMAKE_CXX_STANDARD 14) This is generally frowned upon for two different reasons. First and most important, you set this configuration globally for every target you ...


1

I am not familiar with the Clang sanitization command-line options so I can't give any feedback about those, but regarding the CMake code I would suggest the following CMakeLists.txt file cmake_minimum_required(VERSION 3.13) project(FirstProject C) find_package(Curses REQUIRED) add_executable(${CMAKE_PROJECT_NAME} main.c) target_include_directories(${...


1

In a clean rule, consider removing $(OBJECT_FILES) instead of src/*.o I don't see where LDFLAGS is ever used. I understand that you are not likely to edit include files; however, to be on a safe side you may want to add auto dependency generation: DEP_FILES = $(SOURCE_FILES:.cc=.d) %.d: %.cc $(CC) $(INCLUDES) $(CXXFLAGS) -MM -MT -o $@ $< -include $(...


1

for (std::string line; getline(in, line); ) { This for loop looks weird. It looks weird because it's a while loop where you've taken advantage of the ability to declare a variable. I think it'd be better as a while loop: std::string line; while(getline(in, line)) { It's longer, but doesn't make me go "wait, what?" anymore. // stop writing if non-...


1

I think your changes would definetly improve CHECK_LIBRARY_EXISTS(). If I understand correctly you want to improve upon something like cmake_push_check_state() set(CMAKE_REQUIRED_QUIET ${Readline_FIND_QUIETLY}) check_library_exists("${Readline_LIBRARIES}" rl_initialize "" Readline_rl_initialize) cmake_pop_check_state() with assert_library_function(...


1

Judging from the low readability of runTest(), you may benefit from having a separate testing file, allowing you to improve the readability in some way. It may also allow for additional tests, and you can then provide names for each test (the user may want to know what kinds of tests are available).


Only top voted, non community-wiki answers of a minimum length are eligible