[GitHub] [arrow] kou commented on a change in pull request #9045: ARROW-11180: [Developer] cmake-format pre-commit hook doesn't run

[GitBox <gi...@apache.org>]

kou commented on a change in pull request #9045:
URL: https://github.com/apache/arrow/pull/9045#discussion_r554201859



##########
File path: run-cmake-format.py
##########
@@ -21,58 +21,20 @@
 import pathlib
 import subprocess
 import sys
+import argparse
+from pathlib import Path

Review comment:
       Do wee need them?

##########
File path: run-cmake-format.py
##########
@@ -21,58 +21,20 @@
 import pathlib
 import subprocess
 import sys
+import argparse
+from pathlib import Path
 
 
-patterns = [
-    'cpp/CMakeLists.txt',
-    # Keep an explicit list of files to format as we don't want to reformat
-    # files we imported from other location.
-    'cpp/cmake_modules/BuildUtils.cmake',
-    'cpp/cmake_modules/DefineOptions.cmake',
-    'cpp/cmake_modules/FindArrow.cmake',
-    'cpp/cmake_modules/FindArrowCUDA.cmake',
-    'cpp/cmake_modules/FindArrowDataset.cmake',
-    'cpp/cmake_modules/FindArrowFlight.cmake',
-    'cpp/cmake_modules/FindArrowFlightTesting.cmake',
-    'cpp/cmake_modules/FindArrowPython.cmake',
-    'cpp/cmake_modules/FindArrowPythonFlight.cmake',
-    'cpp/cmake_modules/FindArrowTesting.cmake',
-    'cpp/cmake_modules/FindBrotli.cmake',
-    'cpp/cmake_modules/FindClangTools.cmake',
-    'cpp/cmake_modules/FindFlatbuffersAlt.cmake',
-    'cpp/cmake_modules/FindGLOG.cmake',
-    'cpp/cmake_modules/FindGandiva.cmake',
-    'cpp/cmake_modules/FindInferTools.cmake',
-    'cpp/cmake_modules/FindLLVMAlt.cmake',
-    'cpp/cmake_modules/FindLz4.cmake',
-    'cpp/cmake_modules/FindParquet.cmake',
-    'cpp/cmake_modules/FindPlasma.cmake',
-    'cpp/cmake_modules/FindPython3Alt.cmake',
-    'cpp/cmake_modules/FindRE2.cmake',
-    'cpp/cmake_modules/FindRapidJSONAlt.cmake',
-    'cpp/cmake_modules/FindSnappyAlt.cmake',
-    'cpp/cmake_modules/FindThrift.cmake',
-    'cpp/cmake_modules/FindZSTD.cmake',
-    'cpp/cmake_modules/Findc-aresAlt.cmake',
-    'cpp/cmake_modules/FindgRPCAlt.cmake',
-    'cpp/cmake_modules/FindgflagsAlt.cmake',
-    'cpp/cmake_modules/Findjemalloc.cmake',
-    'cpp/cmake_modules/SetupCxxFlags.cmake',
-    'cpp/cmake_modules/ThirdpartyToolchain.cmake',
-    'cpp/cmake_modules/san-config.cmake',
-    'cpp/cmake_modules/UseCython.cmake',
-    'cpp/src/**/CMakeLists.txt',
-    'cpp/tools/**/CMakeLists.txt',
-    'java/gandiva/CMakeLists.txt',
-    'python/CMakeLists.txt',
-]
+PATTERNS = ['cpp/cmake_modules/*.cmake', '*CMakeLists.txt']
+EXCLUDE = ['cpp/cmake_modules/FindNumPy.cmake',
+'cpp/cmake_modules/FindPythonLibsNew.cmake', 'cpp/cmake_modules/UseCython.cmake']

Review comment:
       ```suggestion
   EXCLUDE = [
       'cpp/cmake_modules/FindNumPy.cmake',
       'cpp/cmake_modules/FindPythonLibsNew.cmake',
       'cpp/cmake_modules/UseCython.cmake',
   ]
   ```
   
   This style will be easy to add/remove items.

##########
File path: run-cmake-format.py
##########
@@ -21,58 +21,20 @@
 import pathlib
 import subprocess
 import sys
+import argparse
+from pathlib import Path
 
 
-patterns = [
-    'cpp/CMakeLists.txt',
-    # Keep an explicit list of files to format as we don't want to reformat
-    # files we imported from other location.

Review comment:
       Could you keep this comment to show why we have the list?




----------------------------------------------------------------
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:
users@infra.apache.org