search

lammertb / libcrc

Multi platform CRC library
MIT License
576 stars 247 forks source link

crc: basic CMake support #22

Open VShilenkov opened 2 years ago

VShilenkov commented 2 years ago

supports CMake build, install, test routines available generate cmake configuration for find_package() generate pkg_config instructions

supports include_what_you_use, clang-tidy, unit testing, generation of examples, code coverage report

lammertb commented 2 years ago

Thanks for the pull request.

Can you explain to me what these more than 1300 undocumented lines add to the functionality of the CRC calculations?

VShilenkov commented 2 years ago

Code of the project ( sources and headers) was untouched. So it doesn't affect functionality. This PR allows all who uses CMake include libcrc to the project in a straightforward way. Also for those who rely on pkg_config ways of resolving dependencies

lammertb commented 2 years ago

So it doesn't add functionality to the CRC functionality, only functionality for project management. And to be honest, I don't like undocumented code.

Without documentation, this PR will not be merged.

VShilenkov commented 2 years ago

cmake lists has inline comments which describes sections and purpose of each block give me an example of how would you like this files to be documented and I will work on that

lammertb commented 2 years ago

It should basically cover the following points

lammertb commented 2 years ago

As an addition: please also look at the INSTALL file in https://github.com/lammertb/libcrc/blob/master/INSTALL which details the specific reasons why the old-fashioned make tool was used in the package, rather than cmake.

VShilenkov commented 2 years ago