Add codespell pre-commit hook #2855
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
🧱 stack: api
openverse-bot
added
🟩 priority: low
|
Size Change: 0 B Total Size: 877 kB ℹ️ View Unchanged
|
sarayourfriend
requested review from
krysal,
stacimc,
AetherUnbound and
dhruvkb
dhruvkb
requested changes
Aug 22, 2023
catalog/tests/dags/providers/provider_api_scripts/test_wikimedia_commons.py
Outdated
Show resolved
Hide resolved
...ety/detecting_sensitive_textual_content/20230309-implementation_plan_sensitive_terms_list.md
Show resolved
Hide resolved
catalog/tests/dags/common/test_resources/image_table_sample.tsv
Outdated
Show resolved
Hide resolved
|
Thank you for the careful review, Dhruv. I'll make updates to this tomorrow! |
sarayourfriend
force-pushed
the
add/spell-checker
branch
from
58f169d
to
c59cdba
Compare
obulat
approved these changes
Aug 23, 2023
dhruvkb
approved these changes
Aug 23, 2023
obulat
pushed a commit
that referenced
this pull request
Aug 25, 2023
* Add minimal codespell config * Flesh out codespell configuration and consolidate python tool configs * Correct codespell errors * Undo bad spelling switch and add ignore for the line * Move codespell docs to docs site * Remove unnecessary custom dictionary entries * Undo errant corrections
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes
Fixes #378 by @dhruvkb
Description
This PR does the following:
Codespell seems fine enough and works pretty well.
However, I would like to also explore using cspell. It has far greater flexibility in terms of the supported dictionaries and its configuration is significantly less obscure than Codespells. I'm going to leave this PR as a draft until I've tried cspell. If I decide to go with cspell, I'll open a new PR and keep the configuration consolidation I did for other Python-based tools in this PR. That seems useful regardless of which spell checker we choose.I tried out cspell and while it probably works great if you have a small project with little prose, it complains about every unknown word. At a glance I could not find anything in cspell's documentation about how to ignore unknown words, I suspect because its strategy for matching words is very different. Codespell is going to be much easier to maintain in the long run. It isn't perfect and will probably miss some things that cspell would catch, but it isn't worth the tremendous task of configuration and maintenance to use cspell. Keep in mind that "unknown words" includes most Vue related terms, all maintainer GitHub usernames (referenced frequently in project documentation), and so on and so on. In any case, Codespell is a good start. If we want to switch tools in the future we can do so. For now this gets us at least catching basic misspellings.Testing Instructions
Checkout the branch and run
just lint codespell. Confirm there are no errors. Add a misspelling to a relevant file (one that isn't ignored) and confirm that codespell catches it when runningjust lint codespell. Also, review the list of ignored files and confirm that it's comprehensive and does not accidentally include files that should be checked.Look through the spelling changes and confirm that there are no problems with the fixes I've applied. There is a mix of automatically applied fixes that codespell generated as well as fixes I had to do manually (for when there were multiple suggestions for a misspelled word).
Finally, read through the configuration documentation and confirm that it makes sense, is clear, and can be used effectively to maintain the configuration moving forward.
Checklist
Update index.md).main) or a parent feature branch.Developer Certificate of Origin
Developer Certificate of Origin