search

iamantony / qtcsv

Library for reading and writing csv-files in Qt.
MIT License
265 stars 141 forks source link

Fix #42 -- Increase CMake requirement to 3.0 and utilize VERSION in project(). #44

Closed apollo13 closed 7 years ago

apollo13 commented 7 years ago

It should even be possible to get rid of the LIB_* variables if wanted.

iamantony commented 7 years ago

Thanks for the PR!

Looks good, but I think we could go further.

  1. CMakeLists.txt 1.1 remove definition of variables: LIB_MAJOR_VERSION, LIB_MINOR_VERSION, LIB_REVISION_VERSION and LIB_VERSION 1.2 in set_target_properties() we could use PROJECT_VERSION and PROJECT_VERSION_MAJOR variables instead of deleted variables

  2. tests/CMakeLists.txt 2.1 Do not remove cmake_minimum_required() and project() 2.2 Remove variable BINARY_NAME and use PROJECT_NAME instead

apollo13 commented 7 years ago
  1. Fully agree, I just wanted your feedback on whether someone might use LIB_* etc…
  2. Why keep cmake_minimum_required/project? It serves no purpose especially since the tests cannot be build on their own (they rely on the existence of upper directory variables).