[5/5,precommit] Add codespell hook
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_gdb_build--master-arm |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-arm |
success
|
Test passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 |
success
|
Test passed
|
Commit Message
Add a pre-commit codespell hook for directories gdbsupport and gdbserver,
which are codespell-clean:
...
$ pre-commit run codespell --all-files
codespell................................................................Passed
...
A non-trivial question is where the codespell configuration goes.
Currently we have codespell sections in gdbsupport/setup.cfg and
gdbserver/setup.cfg, but codespell doesn't automatically use those because the
pre-commit hook runs codespell at the root of the repository.
A solution would be to replace those 2 setup.cfg files with a setup.cfg in the
root of the repository. Not ideal because generally we try to avoid adding
files related to subdirectories at the root.
Another solution would be to add two codespell hooks, one using
--config gdbsupport/setup.cfg and one using --config gdbserver/setup.cfg, and
add a third one once we start supporting gdb. Not ideal because it creates
duplication, but certainly possible.
I went with the following solution: a setup.cfg file in gdb/contrib (alongside
codespell-ignore-words.txt) which is used for both gdbserver and gdbsupport.
So, what can this new setup do for us? Let's demonstrate by simulating a typo:
...
$ echo "/* aways */" >> gdbsupport/agent.cc
...
We can check unstaged changes before committing:
...
$ pre-commit run codespell --all-files
codespell................................................................Failed
- hook id: codespell
- exit code: 65
gdbsupport/agent.cc:282: aways ==> always, away
...
Likewise, staged changes (no need for the --all-files):
...
$ git add gdbsupport/agent.cc
$ pre-commit run codespell
codespell................................................................Failed
- hook id: codespell
- exit code: 65
gdbsupport/agent.cc:282: aways ==> always, away
...
Or we can try to commit, and run into the codespell failure:
...
$ git commit -a
black................................................(no files to check)Skipped
flake8...............................................(no files to check)Skipped
isort................................................(no files to check)Skipped
codespell................................................................Failed
- hook id: codespell
- exit code: 65
gdbsupport/agent.cc:282: aways ==> always, away
check-include-guards.................................(no files to check)Skipped
...
which makes the commit fail.
---
.pre-commit-config.yaml | 6 ++++++
{gdbserver => gdb/contrib}/setup.cfg | 4 +++-
gdbsupport/agent.cc | 1 +
gdbsupport/setup.cfg | 4 ----
4 files changed, 10 insertions(+), 5 deletions(-)
rename {gdbserver => gdb/contrib}/setup.cfg (64%)
delete mode 100644 gdbsupport/setup.cfg
Comments
>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
Tom> I went with the following solution: a setup.cfg file in gdb/contrib (alongside
Tom> codespell-ignore-words.txt) which is used for both gdbserver and gdbsupport.
FWIW I think this approach makes sense.
Tom
Hello Tom,
Tom de Vries <tdevries@suse.de> writes:
> Add a pre-commit codespell hook for directories gdbsupport and gdbserver,
> which are codespell-clean:
> ...
> $ pre-commit run codespell --all-files
> codespell................................................................Passed
> ...
>
> A non-trivial question is where the codespell configuration goes.
>
> Currently we have codespell sections in gdbsupport/setup.cfg and
> gdbserver/setup.cfg, but codespell doesn't automatically use those because the
> pre-commit hook runs codespell at the root of the repository.
>
> A solution would be to replace those 2 setup.cfg files with a setup.cfg in the
> root of the repository. Not ideal because generally we try to avoid adding
> files related to subdirectories at the root.
>
> Another solution would be to add two codespell hooks, one using
> --config gdbsupport/setup.cfg and one using --config gdbserver/setup.cfg, and
> add a third one once we start supporting gdb. Not ideal because it creates
> duplication, but certainly possible.
>
> I went with the following solution: a setup.cfg file in gdb/contrib (alongside
> codespell-ignore-words.txt) which is used for both gdbserver and gdbsupport.
I agree with Tom Tromey that the approach you chose is better.
> So, what can this new setup do for us? Let's demonstrate by simulating a typo:
> ...
> $ echo "/* aways */" >> gdbsupport/agent.cc
> ...
<snip>
> diff --git a/gdbsupport/agent.cc b/gdbsupport/agent.cc
> index c70b59a4c83..be14a57f627 100644
> --- a/gdbsupport/agent.cc
> +++ b/gdbsupport/agent.cc
> @@ -279,3 +279,4 @@ agent_capability_invalidate (void)
> {
> agent_capability = 0;
> }
> +/* aways */
I suppose it's redundant to mention since the precommit hook will
prevent you from committing this patch, but it's better not to upstream
the simulated typo. :)
--
Thiago
@@ -65,6 +65,12 @@ repos:
- id: isort
types_or: [file]
files: 'gdb/.*\.py(\.in)?$'
+ - repo: https://github.com/codespell-project/codespell
+ rev: v2.4.1
+ hooks:
+ - id: codespell
+ files: '^(gdbsupport|gdbserver)/'
+ args: [--config, gdb/contrib/setup.cfg]
- repo: local
hooks:
- id: check-include-guards
similarity index 64%
rename from gdbserver/setup.cfg
rename to gdb/contrib/setup.cfg
@@ -1,4 +1,6 @@
[codespell]
+
# Skip ChangeLogs and generated files.
-skip = ChangeLog*,configure
+skip = */ChangeLog*,*/configure,gdbsupport/Makefile.in
+
ignore-words = gdb/contrib/codespell-ignore-words.txt
@@ -279,3 +279,4 @@ agent_capability_invalidate (void)
{
agent_capability = 0;
}
+/* aways */
deleted file mode 100644
@@ -1,4 +0,0 @@
-[codespell]
-# Skip ChangeLogs and generated files.
-skip = ChangeLog*,Makefile.in,configure
-ignore-words = gdb/contrib/codespell-ignore-words.txt