kou commented on a change in pull request #9045:
URL: https://github.com/apache/arrow/pull/9045#discussion_r550926812
##########
File path: .pre-commit-config.yaml
##########
@@ -40,9 +40,10 @@ repos:
- id: cmake-format
name: CMake Format
language: python
- entry: bash -c "pip install cmake-format && python run-cmake-format.py
--check"
- entry: echo
- files: ^(.*/CMakeLists.txt|.*.cmake)$
+ entry: cmake-format --in-place --autosort=false
Review comment:
We don't want to reformat imported CMake files:
https://github.com/apache/arrow/blob/master/run-cmake-format.py#L28-L29
( @xhochy Do you still want to avoid reformatting imported CMake files? The
comment was added by you at 79c93c77542524042e26761c5107127f51e5c193. )
Could you use `run-cmake-format.py` to keep the configuration for it in one
place?
BTW, we can't maintain the target list by current allow list style. The
`patterns` is out of date. It still has renamed files. Can we change the target
list to deny list style? Here is a list of imported CMake files:
* `cpp/cmake_modules/FindNumPy.cmake`
* `cpp/cmake_modules/FindPythonLibsNew.cmake`
* `cpp/cmake_modules/UseCython.cmake` (Oh... This is already reformatted...)
If `run-cmake-format.py` accepts a target file like `run-cmake-format.py
cpp/CMakeLists.txt` and it processes only the given file, can we remove the
performance difference between `run-cmake-format.py` and `cmake-format`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
