search

pnlbwh / ukftractography

Other
25 stars 27 forks source link

Strip trailing spaces and convert tabs to spaces #157

Open jcfr opened 8 months ago

jcfr commented 8 months ago

Changes originally contributed in https://github.com/pnlbwh/ukftractography/pull/149

jcfr commented 8 months ago

cc: @tashrifbillah @pieper

tashrifbillah commented 8 months ago

Jean, thank you for detecting this style issues. However, is it your cmake convention to use two spaces for tab? Asking because we always use four spaces across Python, MATLAB, Bash, JavaScript.

jcfr commented 8 months ago

The use of 2-space has been the convention applied in CMake itself^1 and all of the CMake-based build-system I have been working with. Moving forward with this change will help developer familiar with maintaining such system.

Once this is integrated, I would also suggest moving forward with adding .pre-commit-config.yaml like we have in Slicer, this will help minimize style discrepancies and enforce consistency throughout the code base.

jcfr commented 8 months ago

Once integrated, these style commits will be added to a file called .git-blame-ignore-revs.

This file lists revisions that should be ignored when considering attribution for the actual code written. Code style changes should not be considered as modifications with regards to attribution.

To see clean and meaningful blame information. 

$ git blame important.py --ignore-revs-file .git-blame-ignore-revs

To configure git to automatically ignore revisions listed in a file on every call to git blame.

$ git config blame.ignoreRevsFile .git-blame-ignore-revs

See https://docs.github.com/en/repositories/working-with-files/using-files/viewing-a-file#ignore-commits-in-the-blame-view

For an example of with/without this file in the context of the GitHub UI, see https://github.com/Slicer/Slicer/pull/6306#issuecomment-1092426901

tashrifbillah commented 7 months ago

Hi @pieper , I installed the latest cmake-3.29.0-rc4 from Kitware website but the bin directory does not contain ccmake:

$ ls cmake-3.29.0-rc4/build/bin/
cmake  cpack  ctest  ctresalloc

Has there been a change in how they are packaging it now? In other words, how can I get ccmake?

pieper commented 7 months ago

Maybe it was an issue with 3.29.0-rc4. 3.29.1 seems to have it:

image

tashrifbillah commented 7 months ago

Hi @pieper , thank you but how did you get that packages?

I downloaded source from https://cmake.org/download/ , and did:

tar -xzf cmake-3.29.1.tar.gz
cd cmake-3.29.1 && mkdir build && cd build
../configure --prefix=`pwd`
gmake -j 4

This is what I always did but the build/bin of 3.29.1 still does not contain ccmake. I tried Googling but couldn't really find a complete building instruction. If you know, can you point me to?

tashrifbillah commented 7 months ago

Hi Steve, I found a README.rst within cmake-3.29.1 directory. I am following that now. Thanks.

pieper commented 7 months ago

Also fwi @tashrifbillah I downloaded the binaries and ccmake was included. You may need to enable ccmake before building. It could be a configure option.

tashrifbillah commented 7 months ago

Hi @pieper and @jcfr , could you tell me what latest GCC version ITK supports? I tried with both GCC 11.4.1 (RHEL 9 default) and GCC 9.5.0. Both are reported as failure by ITK:

"dunno about this GCC"

pieper commented 7 months ago

latest GCC version ITK supports

I don't know off the top of my head. It would be a good question for the ITK forum.