diff mbox series

[v2,1/2] Enable codespell by default. Can be disabled from config file.

Message ID 20190214193547.30783-2-msantana@redhat.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series Minor changes to checkpatches |

Checks

Context Check Description
ci/Intel-compilation success Compilation OK
ci/mellanox-Performance-Testing success Performance Testing PASS
ci/intel-Performance-Testing success Performance Testing PASS

Commit Message

Michael Santana Feb. 14, 2019, 7:35 p.m. UTC 
  Enable codespell by default.
codespell is a feature by checkpatch.pl that
checks for common spelling mistakes in patches.

This feature is disabled by default. To enable it one must add
the '--codespell' flag to the $options variable in
checkpatches.sh. With this change codespell is enabled by default.
The user can decide to turn off codespell from a one of the config
files read by checkpatches.sh.

Signed-off-by: Michael Santana <msantana@redhat.com>
Reviewed-by: Rami Rosen <ramirose@gmail.com>
---
 devtools/checkpatches.sh | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Thomas Monjalon Feb. 28, 2019, 11:21 a.m. UTC | #1 
14/02/2019 20:35, Michael Santana:
> Enable codespell by default.
> codespell is a feature by checkpatch.pl that
> checks for common spelling mistakes in patches.

What is the difference between codespell and spelling.txt included
with checkpatch?
Is it just a different dictionary?

> This feature is disabled by default. To enable it one must add
> the '--codespell' flag to the $options variable in
> checkpatches.sh.

We need also to specify the dictionary path if not in
/usr/share/codespell/dictionary.txt
In my case, it is in /usr/lib/python3.7/site-packages/codespell_lib/data/dictionary.txt

> With this change codespell is enabled by default.

It seems it is not enabled by default,
because we need DPDK_CHECKPATCH_CODESPELL=enable

> The user can decide to turn off codespell from a one of the config
> files read by checkpatches.sh.
[...]
>  # override default Linux options
>  options="--no-tree"
> +if [ "$DPDK_CHECKPATCH_CODESPELL" == "enable" ]; then

What about allowing either "enable" or a path?
If it is a path (have some slash), then we can add --codespellfile option.

> +    options="$options --codespell"
> +fi
>  options="$options --max-line-length=$length"
>  options="$options --show-types"
>  options="$options --ignore=LINUX_VERSION_CODE,\
>
Michael Santana Feb. 28, 2019, 10:09 p.m. UTC | #2 
On 2/28/19 6:21 AM, Thomas Monjalon wrote:
> 14/02/2019 20:35, Michael Santana:
>> Enable codespell by default.
>> codespell is a feature by checkpatch.pl that
>> checks for common spelling mistakes in patches.
> What is the difference between codespell and spelling.txt included
> with checkpatch?
> Is it just a different dictionary?
codespell has a larger dictionary, about 15000 word fixes whereas 
spelling.txt has about 1000.
That's really the only big difference
>
>> This feature is disabled by default. To enable it one must add
>> the '--codespell' flag to the $options variable in
>> checkpatches.sh.
> We need also to specify the dictionary path if not in
> /usr/share/codespell/dictionary.txt
> In my case, it is in /usr/lib/python3.7/site-packages/codespell_lib/data/dictionary.txt
>
>> With this change codespell is enabled by default.
> It seems it is not enabled by default,
> because we need DPDK_CHECKPATCH_CODESPELL=enable
V2 sets DPDK_CHECKPATCH_CODESPELL=enable at the beginning of 
checkpatches, right before reading in the config files.
If DPDK_CHECKPATCH_CODESPELL is set in one of the config files it 
overwrites the enabled by default.
This way a user can disable it via a config file
>
>> The user can decide to turn off codespell from a one of the config
>> files read by checkpatches.sh.
> [...]
>>   # override default Linux options
>>   options="--no-tree"
>> +if [ "$DPDK_CHECKPATCH_CODESPELL" == "enable" ]; then
> What about allowing either "enable" or a path?
> If it is a path (have some slash), then we can add --codespellfile option.
I like your thinking. We can use `if [ -f <file> ]` to see if the path 
given is an existing file.
so, if DPDK_CHECKPATCH_CODESPELL is set to enable, then enable it with 
default path (the way it is right now)
if DPDK_CHECKPATCH_CODESPELL is set to a valid path to a file then 
enable codespell and set --codespellfile to said file
otherwise if it's not set to enable or set to a valid path file, then 
assume it's disabled.
Missed anything?
>
>> +    options="$options --codespell"
>> +fi
>>   options="$options --max-line-length=$length"
>>   options="$options --show-types"
>>   options="$options --ignore=LINUX_VERSION_CODE,\
>>
>
>
>
>
Thomas Monjalon Feb. 28, 2019, 11:01 p.m. UTC | #3 
28/02/2019 23:09, Michael Santana Francisco:
> On 2/28/19 6:21 AM, Thomas Monjalon wrote:
> > 14/02/2019 20:35, Michael Santana:
> >> Enable codespell by default.
> >> codespell is a feature by checkpatch.pl that
> >> checks for common spelling mistakes in patches.
> > What is the difference between codespell and spelling.txt included
> > with checkpatch?
> > Is it just a different dictionary?
> codespell has a larger dictionary, about 15000 word fixes whereas 
> spelling.txt has about 1000.
> That's really the only big difference

OK

> >> This feature is disabled by default. To enable it one must add
> >> the '--codespell' flag to the $options variable in
> >> checkpatches.sh.
> > We need also to specify the dictionary path if not in
> > /usr/share/codespell/dictionary.txt
> > In my case, it is in /usr/lib/python3.7/site-packages/codespell_lib/data/dictionary.txt
> >
> >> With this change codespell is enabled by default.
> > It seems it is not enabled by default,
> > because we need DPDK_CHECKPATCH_CODESPELL=enable
> V2 sets DPDK_CHECKPATCH_CODESPELL=enable at the beginning of 
> checkpatches, right before reading in the config files.
> If DPDK_CHECKPATCH_CODESPELL is set in one of the config files it 
> overwrites the enabled by default.
> This way a user can disable it via a config file

OK I missed it.

> >> The user can decide to turn off codespell from a one of the config
> >> files read by checkpatches.sh.
> > [...]
> >>   # override default Linux options
> >>   options="--no-tree"
> >> +if [ "$DPDK_CHECKPATCH_CODESPELL" == "enable" ]; then
> > What about allowing either "enable" or a path?
> > If it is a path (have some slash), then we can add --codespellfile option.
> I like your thinking. We can use `if [ -f <file> ]` to see if the path 
> given is an existing file.
> so, if DPDK_CHECKPATCH_CODESPELL is set to enable, then enable it with 
> default path (the way it is right now)
> if DPDK_CHECKPATCH_CODESPELL is set to a valid path to a file then 
> enable codespell and set --codespellfile to said file
> otherwise if it's not set to enable or set to a valid path file, then 
> assume it's disabled.
> Missed anything?

Well described. Would be nice to see in v3. Thanks
diff mbox series

Patch

diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
index 3b03b7ef2..9c2b0a28a 100755
--- a/devtools/checkpatches.sh
+++ b/devtools/checkpatches.sh
@@ -2,9 +2,12 @@ 
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright 2015 6WIND S.A.
 
+# Enable codespell by default. This can be overwritten from a config file.
+DPDK_CHECKPATCH_CODESPELL=enable
 # Load config options:
 # - DPDK_CHECKPATCH_PATH
 # - DPDK_CHECKPATCH_LINE_LENGTH
+# - DPDK_CHECKPATCH_CODESPELL
 . $(dirname $(readlink -e $0))/load-devel-config
 
 VALIDATE_NEW_API=$(dirname $(readlink -e $0))/check-symbol-change.sh
@@ -13,6 +16,9 @@  length=${DPDK_CHECKPATCH_LINE_LENGTH:-80}
 
 # override default Linux options
 options="--no-tree"
+if [ "$DPDK_CHECKPATCH_CODESPELL" == "enable" ]; then
+    options="$options --codespell"
+fi
 options="$options --max-line-length=$length"
 options="$options --show-types"
 options="$options --ignore=LINUX_VERSION_CODE,\