Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modernize and clean up CMakeLists.txt for streets_service_base #236

Open
1 of 3 tasks
adamlm opened this issue Oct 15, 2022 · 0 comments · May be fixed by #237
Open
1 of 3 tasks

Modernize and clean up CMakeLists.txt for streets_service_base #236

adamlm opened this issue Oct 15, 2022 · 0 comments · May be fixed by #237

Comments

@adamlm
Copy link
Contributor

adamlm commented Oct 15, 2022

Types of Issue

  • Anomaly report (something appears to not work correctly)
  • Enhancement request (describe the enhancement being requested)
  • Other (please ensure the description clarifies why the issue doesn’t fall into either of the above categories)

Descriptive summary

The CMakeLists.txt file for the streets_service_base project could be modernized and cleaned up some. The following are some areas for improvement based on Craig Scott's Professional CMake book:

  • more recent CMake version. Ubuntu 20.04 comes pre-installed with version 3.16, which is still several minor versions behind (latest is 3.24). By upgrading versions, we can utilize more modern features.
  • remove hard coded paths. The installation locations use hard coded paths, which can make customizing the installation location more challenging. Modern CMake recommends using CMAKE_INSTALL_ variables.
  • add configuration options. Some options, such as spdlog logging levels and whether to build tests, are hard coded. A more flexible approach would be to delegate those decisions to consumers using options.
  • general readability improvements. The CMakeLists.txt file uses a lot of variables, but they can be excessive for a project of this size. Using more variables also makes the project harder to read/understand, especially for newcomers.

Carma streets version where this issue was discovered

The develop branch

Expected behavior

N/A

Actual behavior

N/A

Steps to reproduce the actual behavior

N/A

Related work

@adamlm adamlm linked a pull request Oct 15, 2022 that will close this issue
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant