scikit-hep / fastjet

Jet-finding in the Scikit-HEP ecosystem.
https://fastjet.readthedocs.io
BSD 3-Clause "New" or "Revised" License
21 stars 14 forks source link

Run clang-tidy (or clang-format?) on src/_ext.cpp #124

Closed jpivarski closed 2 years ago

jpivarski commented 2 years ago

There are loops in src/_ext.cpp that aren't indented, which would make it a lot easier to follow their logic. If this were Python, we could just run black on all the code (well, if this were Python, the wrong-indentation would be syntactically wrong).

https://github.com/scikit-hep/fastjet/blob/f12e4ea22ac64b2bc9094d1055e8ee8d7da6dd8f/src/_ext.cpp#L134-L146

Is there an equivalent with clang-tidy (or clang-format)?

That is, no meticulous setup, specifying which rules you want to use or not use, just run it and the code is better? Perhaps it would be ideal to have it in CI, but even that is overkill: running it once and getting everything indented once would make a huge impact on readability, even if it isn't enforced for future commits and slowly decays. (Then just run it again!)

henryiii commented 2 years ago

even if it isn't enforced for future commits and slowly decays. (Then just run it again!)

Just enforce it with pre-commit. It'll fix it and you'll never have to worry about poorly formatted code again.

And yes, the tool is clang-format, there's a pre-commit check in skhep dev pages, and you can use -style=llvm if you want no setup. Or just grab the .clang-format file from boost-histogram.

jpivarski commented 2 years ago

I wanted the request to sound really easy, so that it wouldn't be postponed due to not being sure how to add something to pre-commit. Running it once would be life-changing; running it in every commit would be even better.

chrispap95 commented 2 years ago

I was looking about this in the scikit-hep documentation earlier today. The fix seems like this

- repo: https://github.com/pre-commit/mirrors-clang-format
  rev: "v14.0.6"
  hooks:
    - id: clang-format
      types_or: [c++, c, cuda]

plus a .clang-format file. Should we try this?