villano-lab / nrCascadeSim

calculating the NR spectrum resulting from neutron-capture cascades.
MIT License
0 stars 1 forks source link

[JOSS Review] Consider using modern CMake patterns #101

Open matthewfeickert opened 2 years ago

matthewfeickert commented 2 years ago

I am submitting a documentation issue.

The file(s) in question is/are:

I'll link to them below using the commit hash to get a render and permalink.

The problem is in the following category/categories:

Description of the problem: Describe whatever is wrong with the documentation or could otherwise be improved.

The v1.4.0 Download & Build docs suggest to use the legacy in source build pattern and to change to Make right away

https://github.com/villano-lab/nrCascadeSim/blob/83fac469648dc940585c9eee64dbe9488e0fe434/docs/source/01_Getting_Started.rst?plain=1#L84-L88

While not wrong, the legacy pattern isn't needed and even on old/stability focused operating systems like CentOS 7 in almost every situation in which software development is happening you'll have access to devtoolset-7 or newer in which cmake(3) is available. So I'd recommend using the pattern of

$ cmake <options> -S <path to source> -B <path to build directory to make>
$ cmake <path to build directory> -LH  # See all of the options you've selected in a human readable form before build
$ cmake --build <path to build directory> --clean-first --parallel $(($(nproc) - 1))

as this gives the benefits of out-of-source builds, more transparency on what is happening, and allows for other build systems to be used other than Make (e.g. Ninja).

For more on this please see the book An Introduction to Modern CMake by Henry Schreiner.

nuclearGoblin commented 1 year ago

(Added label as we should keep the pattern used in CI the same as that used in documentation.)