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

codespell: config, workflow and some typo fixes #36687

Open
wants to merge 13 commits into
base: develop
Choose a base branch
from

Conversation

yarikoptic
Copy link

Excluded .py files entirely for this initial pass. Someone can remove skipping them and improve there as well

@spackbot-app spackbot-app bot added core PR affects Spack core functionality defaults gitlab Issues related to gitlab integration shell-support tests General test capability(ies) workflow labels Apr 6, 2023
@wdconinc
Copy link
Contributor

Rebased and conflicts resolved (file removed).

@wdconinc wdconinc self-assigned this May 18, 2024
@spackbot-app spackbot-app bot added documentation Improvements or additions to documentation new-variant labels May 18, 2024
Copy link
Contributor

@wdconinc wdconinc left a comment

Choose a reason for hiding this comment

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

Thanks for this PR! Apologies for the delayed review.

Your PR is reasonable scoped (i.e. doesn't include packages at this stage), which I think is the right approach.

.github/workflows/codespell.yml Outdated Show resolved Hide resolved
.github/workflows/codespell.yml Outdated Show resolved Hide resolved
yarikoptic and others added 11 commits May 20, 2024 09:11
…tions

Co-authored-by: Wouter Deconinck <wdconinc@gmail.com>
=== Do not change lines below ===
{
 "chain": [],
 "cmd": "codespell -w -i 3 -C 2",
 "exit": 0,
 "extra_inputs": [],
 "inputs": [],
 "outputs": [],
 "pwd": "."
}
^^^ Do not change lines above ^^^
=== Do not change lines below ===
{
 "chain": [],
 "cmd": "codespell -w",
 "exit": 0,
 "extra_inputs": [],
 "inputs": [],
 "outputs": [],
 "pwd": "."
}
^^^ Do not change lines above ^^^
@yarikoptic
Copy link
Author

ok, should be good now -- ignored patches.* altogether, and did few other tune ups and rebased again so it is all uptodate.

.codespellrc Outdated Show resolved Hide resolved
@alecbcs
Copy link
Member

alecbcs commented May 20, 2024

I would push for the new workflow be an extra step inside of the existing style checks rather than its own standalone CI.
(Or at least reuse our existing infrastructure inside of workflows/ci.yml as an additional precheck.)

@wdconinc
Copy link
Contributor

I would push for the new workflow be an extra step inside of the existing style checks rather than its own standalone CI.

Suggestion: add to https://github.com/spack/spack/blob/develop/.github/workflows/valid-style.yml after validate, before style?

@adamjstewart
Copy link
Member

I kind of wonder if typo checking is more useful as a warning on release branches than as an error on every commit. I'm still concerned about false positives, but I guess we would have to run it on the entire repo to see how many of those there are.

@yarikoptic
Copy link
Author

yarikoptic commented May 21, 2024

FWIW, we often have codespell even in pre-commit configuration so no typos even get committed. IMHO the sooner they are defected and fixed the better.
False positives can happen but rare and can be whitelisted.

Decide where to plug the action and I will do or feel free to push such a commit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core PR affects Spack core functionality defaults documentation Improvements or additions to documentation gitlab Issues related to gitlab integration new-variant shell-support tests General test capability(ies) workflow
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants