Open LecrisUT opened 11 months ago
Hi @LecrisUT, thanks for proposing your help. To avoid wasting your time, I recommend you do not start a big refactoring effort. Start with something small.
For the moment, you do not know about our testing process, and all of the use cases of CGAL. With a big pull-request, you would a higher risk of breaking several tests and use cases. That would block the integration of the pull-request. With smaller steps, the chance to get them integrated is higher.
I am sorry if this issue is a bit big, but there are a lot of inter-connected components that I want to address. Primarily this comes from some discussions with Laurent on Fedora Bugzilla and doing a quick look at the project structure. So a few points that I want to raise, primarily about the CMake project structure.
Find<Package>.cmake
. If there is a need for compatibility layer, e.g. for fallback topkg-config
, try a structure like this. I have a project that I am developing, and the project is still flexible right now to accommodate for more needs.FetchContent
particularly in combination withFIND_PACKAGE_ARGS
as much as possibleFetchContent
. This means namespacing options, aliasing targets and exporting equivalent variablesctest
testsuite. A simple format is to usectest --build-and-test
to call ctest recursively. This also makes it possible to symlink examples to test directly.CGalConfig.cmake
. Rely on the built-in support for targets to pull in appropriate dependencies.include_directories
etc. Use target scoped ones insteadcompat
folder, with specificversion
guarding and with plans for removal and deprecation following OS lts releasesinclude
, etc. in theCGalConfig.cmake
. This is propagated to all downstream users of CGal, e.g.FindEigen3.cmake
overrides the use system installedEigen3Config.cmake
.FetchContent
logic settingFETCHCONTENT_SOURCE_DIR_<uppercaseName>
to the relative path from the top-levelPROJECT_SOURCE_DIR
src
/docs
/test
/example
(maybeinclude
) folder structure. The internal folder structure can be enforced via the CIpyproject.toml
for visibility and ease of use, even if these are meant for development and CI. Use appropriateentry_points
instead of shebangs.pre-commit
readthedocs
codecov
clang-tidy
qodana
), even for development scriptspackit
I have done quite a bit of CMake implementation and modernization across multiple projects so I can come up with a PR addressing all of these issues over a weekend, if the team is onboard with such refactoring.