Codespell #994
Codespell #994
Conversation
Signed-off-by: kluszcze <[email protected]>
Signed-off-by: kluszcze <[email protected]>
| set -ex | ||
|
|
||
| SCAN_RESULT=`codespell -H --quiet-level=3 --skip="*.h,*.cmake,*.c,*.hpp,*.cpp,*.sh,*.py,test/supp/*.supp,./.venv" --ignore-words-list="ASSER,Tne,ba,BA"` | ||
| echo -e "${SCAN_RESULT}" |
There was a problem hiding this comment.
I don't think we need this extra var SCAN_RESULT - codespell should just print its output to stdout, right?
| @@ -57,6 +57,9 @@ jobs: | |||
| with: | |||
| config: ./.github/workflows/.spellcheck-conf.toml | |||
|
|
|||
| - name: Run codespell | |||
| run: | | |||
| bash .github/scripts/codespell.sh | |||
There was a problem hiding this comment.
no need for bash part, its already done thanks to shebang in your script (#!/bin/bash)
| @@ -57,6 +57,9 @@ jobs: | |||
| with: | |||
| config: ./.github/workflows/.spellcheck-conf.toml | |||
|
|
|||
| - name: Run codespell | |||
| run: | | |||
There was a problem hiding this comment.
you can drop | if it's a one line command call and fit it all in line with run:
|
|
||
| set -ex | ||
|
|
||
| SCAN_RESULT=`codespell -H --quiet-level=3 --skip="*.h,*.cmake,*.c,*.hpp,*.cpp,*.sh,*.py,test/supp/*.supp,./.venv" --ignore-words-list="ASSER,Tne,ba,BA"` |
There was a problem hiding this comment.
why --quiet-level=3? I guess it can be verbose
|
|
||
| set -ex | ||
|
|
||
| SCAN_RESULT=`codespell -H --quiet-level=3 --skip="*.h,*.cmake,*.c,*.hpp,*.cpp,*.sh,*.py,test/supp/*.supp,./.venv" --ignore-words-list="ASSER,Tne,ba,BA"` |
There was a problem hiding this comment.
is there like a lot issues in these files? why skipping so much?
for sure we want to skip *.supp,./.venv, the others, I'm not sure
you can shorten the list of skipped files and not fix any issues. The CI will fail then and we could observe the output of this script and make the decision what to do with issues (if any)
| @@ -0,0 +1,6 @@ | |||
| #!/bin/bash | |||
|
|
|||
There was a problem hiding this comment.
pls add copyright and license (similarly to https://github.com/oneapi-src/unified-memory-framework/blob/main/.github/scripts/get_system_info.sh#L4 )
| @@ -0,0 +1,6 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
you can rename the script file, to have a meaningful name, e.g. run-codespell.sh
|
|
||
| set -ex | ||
|
|
||
| SCAN_RESULT=`codespell -H --quiet-level=3 --skip="*.h,*.cmake,*.c,*.hpp,*.cpp,*.sh,*.py,test/supp/*.supp,./.venv" --ignore-words-list="ASSER,Tne,ba,BA"` |
There was a problem hiding this comment.
If this is just one line to trigger, I don't see why it has to be a separate script instead of having the command directly in the workflow run command.
If it were to be used by developers apart from CI, it should be a python script so it could be run both on Windows and Linux.
There was a problem hiding this comment.
We decided with @lplewa that the script is good, can be re-used.
As for Windows, is it supported? if so, perhaps we should write that script in python...
|
|
||
| set -ex | ||
|
|
||
| SCAN_RESULT=`codespell -H --quiet-level=3 --skip="*.h,*.cmake,*.c,*.hpp,*.cpp,*.sh,*.py,test/supp/*.supp,./.venv" --ignore-words-list="ASSER,Tne,ba,BA"` |
There was a problem hiding this comment.
Are there plenty of false-positives in source code files, like i.e. .h, .c, .cpp? This spell checker would be very useful in these files, too.
Description
Checklist