Currently, there is no tool that will check the various cspell lines in core to see if they are still needed. No does cspell offer such a thing. Although there is an issue, https://github.com/streetsidesoftware/cspell/issues/4150.
Most of the lines in core are cspell:ignore with a few cspell:disable-next-line. The latter are near code being changed so is more likely to be kept up to date. However, the ignore lines are at the top of the file and are easy to get out of sync.
Add a check in commit-code-check.sh to report if any of the words in a cspell:ignore line in any of the modified files can be removed.
Review
Manual testing
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | 3390959-2.patch | 1.52 KB | quietone |
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
compare
Comments
Comment #2
quietone commentedI applied the patch and ran the script for two cases. The following shows the relevant part of the output.
1) Add a word that is not used to an ignore line.
1) Add a comment to a file that has an ignore line.
No need to run tests but setting to NR.
Comment #3
quietone commentedComment #4
smustgrave commentedVerified this was working.
Applied patch #2
In action.module I added // cspell:ignore addedline which is a word in the dictionary.txt
Running commit-code-check.sh I got
Removing the line from action.module and running again was all green.
Comment #5
alexpottWe need to update the gitlab-ci pipeline to do this otherwise only committers might run this check.
Comment #6
quietone commentedI agree that this needs to be run in gitlab. However, I think that part is better that it is done as part of or after this issue, #3386841: Reconcile gitlab lint jobs and commit-code-check.sh. That issue is proposing breaking up commit-code-check into several scripts that can be run in both environments. So, this would just be incorporating into one of those new scripts instead of direct additions to the pipeline.
Comment #7
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue.
While you are making the above changes, we recommend that you convert this patch to a merge request. Merge requests are preferred over patches. Be sure to hide the old patch files as well. (Converting an issue to a merge request without other contributions to the issue will not receive credit.)
Comment #8
quietone commentedLet's get a response to #6 before changing to an MR or other work on the code.
Comment #9
smustgrave commentedBrought this up in #needs-review-queue-initiative channel and @alexpott mentioned this should be added to pipeline.