Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add codespell support (config, workflow to detect/not fix) and make it fix few typos #492

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

yarikoptic
Copy link

More about codespell: https://github.com/codespell-project/codespell .

I personally introduced it to dozens if not hundreds of projects already and so far only positive feedback.

CI workflow has 'permissions' set only to 'read' so also should be safe.

@manics
Copy link
Member

manics commented Jul 12, 2024

Is it possible to use codespell with just pre-commit, so we don't need another GitHub workflow?

@yarikoptic yarikoptic marked this pull request as draft July 12, 2024 19:39
@yarikoptic
Copy link
Author

yarikoptic commented Jul 12, 2024

Is it possible to use codespell with just pre-commit, so we don't need another GitHub workflow?

yes*! I just was not sure if any of existing workflows already would run pre-commit on changes -- I pushed TEMP commit with a typo, let's see if gets picked up... so far I do not see any other CI step raising an error...

* codespell workflow also includes "matcher" step which annotates PR diff on where&which typo has happened like did now:

image

if we remove workflow - that would be gone - but it would just remove that convenience.

@manics
Copy link
Member

manics commented Jul 16, 2024

We can deal with enforcing pre-commit in #489

I think keeping things simple and using just pre-commit is best, especially as typos should be infrequent after an initial review of the repo.

=== Do not change lines below ===
{
 "chain": [],
 "cmd": "codespell -w",
 "exit": 0,
 "extra_inputs": [],
 "inputs": [],
 "outputs": [],
 "pwd": "."
}
^^^ Do not change lines above ^^^
@yarikoptic yarikoptic marked this pull request as ready for review July 18, 2024 03:12
@yarikoptic
Copy link
Author

Coolio, your call, rebased kicking away action and the TEMP commit with a typo, took out of draft.

@manics manics closed this Jul 18, 2024
@manics manics reopened this Jul 18, 2024
Copy link
Member

@manics manics left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Ignore the pre-commit failure since it's due to pre-exisitng formatting issues, codespell is passing.

# Ref: https://github.com/codespell-project/codespell#using-a-config-file
skip = '.git*,*.svg,*.lock'
check-hidden = true
ignore-regex = '\bTE\b'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a comment for why this is needed (presumably you've found some intentional use of TE?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants