[PATCH 0/6] [pre-commit] Clean up .pre-commit-config.yaml

[PATCH 0/6] [pre-commit] Clean up .pre-commit-config.yaml

[Tom de Vries]

A few patches dropping unnecessary settings, followed by a final patch
changing the layout a bit.

Tom de Vries (6):
  [pre-commit] Drop types_or filtering for tclint
  [pre-commit] Drop require_serial for pre-commit-setup
  [pre-commit] Remove pass_filenames setting for check-whitespace
  [pre-commit] Remove stages setting for two hooks
  [pre-commit] Drop always_run setting for codespell-log
  [pre-commit] Reorganize .pre-commit-config.yaml

 .pre-commit-config.yaml | 68 ++++++++++++++++++++++-------------------
 1 file changed, 36 insertions(+), 32 deletions(-)


base-commit: 7728a7b19010f6ec0912c5ca69f2b1747c3c2cf5
-- 
2.51.0

[PATCH 1/6] [pre-commit] Drop types_or filtering for tclint

[Tom de Vries]

The pre-commit tclint hook filters files using:
...
      types_or: [file]
      files: '^gdb/testsuite/.*\.(exp|tcl)$'
...

The types_or setting overrides the default file type of the hook.  In the case
of tclint, there's no such default.

So this setting is not required, and causes additional filtering to run.

It was probably added by using one of the python hooks as a starting point.

Remove it.
---
 .pre-commit-config.yaml | 1 -
 1 file changed, 1 deletion(-)

diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index a5b526b1eec..902e58d4d8c 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -123,5 +123,4 @@ repos:
     hooks:
     - id: tclint
       args: [--trust-plugins]
-      types_or: [file]
       files: '^gdb/testsuite/.*\.(exp|tcl)$'
-- 
2.51.0

[PATCH 2/6] [pre-commit] Drop require_serial for pre-commit-setup

[Tom de Vries]

Now that we have pass_filenames=false for pre-commit hook pre-commit-setup,
there's no longer a requirement to have require_serial=true.

Remove the setting.
---
 .pre-commit-config.yaml | 1 -
 1 file changed, 1 deletion(-)

diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index 902e58d4d8c..e5327cbae76 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -111,7 +111,6 @@ repos:
       entry: gdb/contrib/pre-commit-setup.py
       additional_dependencies: ["pyyaml"]
       always_run: true
-      require_serial: true
       pass_filenames: false
     - id: check-file-mode
       name: check-file-mode
-- 
2.51.0

[PATCH 3/6] [pre-commit] Remove pass_filenames setting for check-whitespace

[Tom de Vries]

The pre-commit hook check-whitespace has the following setting:
...
pass_filenames: true
...

Since true is the default for pass_filenames, remove the superfluous setting.
---
 .pre-commit-config.yaml | 1 -
 1 file changed, 1 deletion(-)

diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index e5327cbae76..5be2034b26f 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -103,7 +103,6 @@ repos:
       language: unsupported_script
       entry: gdb/contrib/check-whitespace-pre-commit.py
       files: '^(gdb(support|server)?)/.*$'
-      pass_filenames: true
       stages: [pre-commit]
     - id: pre-commit-setup
       name: pre-commit-setup
-- 
2.51.0

[PATCH 4/6] [pre-commit] Remove stages setting for two hooks

[Tom de Vries]

Some hooks have a setting:
...
stages: [pre-commit]
...
but since that's identical to the default setting:
...
default_stages: [pre-commit]
...
we can just remove those.
---
 .pre-commit-config.yaml | 2 --
 1 file changed, 2 deletions(-)

diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index 5be2034b26f..b250cb1aea6 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -97,13 +97,11 @@ repos:
       files: '^(gdb(support|server)?)/.*\.(c|h|cc)$'
       exclude: '.*/testsuite/.*'
       verbose: true
-      stages: [pre-commit]
     - id: check-whitespace
       name: check-whitespace
       language: unsupported_script
       entry: gdb/contrib/check-whitespace-pre-commit.py
       files: '^(gdb(support|server)?)/.*$'
-      stages: [pre-commit]
     - id: pre-commit-setup
       name: pre-commit-setup
       language: python
-- 
2.51.0

[PATCH 5/6] [pre-commit] Drop always_run setting for codespell-log

[Tom de Vries]

The pre-commit codespell-log hook has an always_run=true setting.

This doesn't seem necessary for a commit-msg stage hook.  Remove it.
---
 .pre-commit-config.yaml | 1 -
 1 file changed, 1 deletion(-)

diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index b250cb1aea6..1d766189ca3 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -77,7 +77,6 @@ repos:
       name: codespell-log
       entry: gdb/contrib/codespell-log.sh
       args: [--toml, gdb/pyproject.toml]
-      always_run: true
       verbose: true
       stages: [commit-msg]
   - repo: local
-- 
2.51.0

[PATCH 6/6] [pre-commit] Reorganize .pre-commit-config.yaml

[Tom de Vries]

Reorganize .pre-commit-config.yaml:
- unify indentation
- move local hooks to last
- add empty lines and header comments to make file more readable
- move detailed file selection comment from flake8 settings to
  "Python hooks" since it's valid for all python hooks, and
  explain why the types_or setting is needed.
- use anchors to avoid repetition
---
 .pre-commit-config.yaml | 62 ++++++++++++++++++++++++-----------------
 1 file changed, 36 insertions(+), 26 deletions(-)

diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index 1d766189ca3..141451501ed 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -40,25 +40,29 @@
 minimum_pre_commit_version: 4.5.1
 default_install_hook_types: [pre-commit, commit-msg]
 default_stages: [pre-commit]
+
 repos:
+  # Python hooks.  Run these for (in glob notation):
+  #
+  #  - gdb/gdb-gdb.py.in
+  #  - gdb/*.py
+  #  - gdb/python/**/*.py
+  #  - gdb/testsuite/*.py
+  #
+  # Because gdb/gdb-gdb.py.in is not classified as python, we use
+  # 'types_or: [file]' to override the default type for these hooks.
+  #
   - repo: https://github.com/psf/black-pre-commit-mirror
     rev: 26.5.1
     hooks:
-      - id: black
-        types_or: [file]
-        files: '^gdb/.*\.py(\.in)?$'
+    - id: black
+      types_or: [file]
+      files: '^gdb/.*\.py(\.in)?$'
   - repo:  https://github.com/pycqa/flake8
     rev: 7.3.0
     hooks:
     - id: flake8
       types_or: [file]
-      # Run this for (in glob notation):
-      #
-      #  - gdb/gdb-gdb.py.in
-      #  - gdb/*.py
-      #  - gdb/python/**/*.py
-      #  - gdb/testsuite/*.py
-      #
       files: '^gdb/.*\.py(\.in)?$'
       args: [--config, gdb/setup.cfg]
   - repo: https://github.com/pycqa/isort
@@ -67,6 +71,8 @@ repos:
     - id: isort
       types_or: [file]
       files: '^gdb/.*\.py(\.in)?$'
+
+  # Codespell hooks.
   - repo: https://github.com/codespell-project/codespell
     rev: v2.4.2
     hooks:
@@ -79,43 +85,47 @@ repos:
       args: [--toml, gdb/pyproject.toml]
       verbose: true
       stages: [commit-msg]
+
+  # Tclint hooks.
+  - repo: https://github.com/nmoroze/tclint
+    rev: v0.8.0
+    hooks:
+    - id: tclint
+      args: [--trust-plugins]
+      files: '^gdb/testsuite/.*\.(exp|tcl)$'
+
+  # Local hooks.
   - repo: local
     hooks:
-    - id: check-include-guards
-      name: check-include-guards
+    - id: &id0 check-include-guards
+      name: *id0
       language: unsupported_script
       entry: gdb/check-include-guards.py
       # All gdb header files, but not headers in the test suite.
       files: '^(gdb(support|server)?)/.*\.h$'
       exclude: '.*/testsuite/.*'
-    - id: check-gnu-style
-      name: check-gnu-style
+    - id: &id1 check-gnu-style
+      name: *id1
       language: python
       additional_dependencies: ['termcolor', 'unidiff']
       entry: gdb/contrib/check-gnu-style-pre-commit.sh
       files: '^(gdb(support|server)?)/.*\.(c|h|cc)$'
       exclude: '.*/testsuite/.*'
       verbose: true
-    - id: check-whitespace
-      name: check-whitespace
+    - id: &id2 check-whitespace
+      name: *id2
       language: unsupported_script
       entry: gdb/contrib/check-whitespace-pre-commit.py
       files: '^(gdb(support|server)?)/.*$'
-    - id: pre-commit-setup
-      name: pre-commit-setup
+    - id: &id3 pre-commit-setup
+      name: *id3
       language: python
       entry: gdb/contrib/pre-commit-setup.py
       additional_dependencies: ["pyyaml"]
       always_run: true
       pass_filenames: false
-    - id: check-file-mode
-      name: check-file-mode
+    - id: &id4 check-file-mode
+      name: *id4
       language: unsupported_script
       entry: gdb/contrib/check-file-mode.sh
       files: '^(gdb|gdbserver|gdbsupport)/.*$'
-  - repo: https://github.com/nmoroze/tclint
-    rev: v0.8.0
-    hooks:
-    - id: tclint
-      args: [--trust-plugins]
-      files: '^gdb/testsuite/.*\.(exp|tcl)$'
-- 
2.51.0

Re: [PATCH 0/6] [pre-commit] Clean up .pre-commit-config.yaml

[Tom Tromey]

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> A few patches dropping unnecessary settings, followed by a final patch
Tom> changing the layout a bit.

This all seems fine to me fwiw.
Approved-By: Tom Tromey <tom@tromey.com>

Tom