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 check-spelling workflow #1057

Merged
merged 1 commit into from
Dec 30, 2021
Merged

Add check-spelling workflow #1057

merged 1 commit into from
Dec 30, 2021

Conversation

jsoref
Copy link
Contributor

@jsoref jsoref commented Dec 30, 2021

1. Issue, if available:

Adds check-spelling.dev workflow (based on check-spelling/spell-check-this@prerelease) per @ellistarn #1045 (comment).

2. Description of changes:

This adds a workflow .github/workflows/spelling.yml with two jobs:

  • spelling this checks the spelling of both branches (via push) and pull requests (via pull_request_target) -- it runs with limited permissions because it takes untrusted input
  • comment this runs when the spelling job fails and has produced output for use to construct a comment -- it needs write permission in order to post a comment (either to the repository or the pull request)

Each time it runs, it will check the spelling of the branch/pull request.

The files in the .github/actions/spelling directory control what it checks (or doesn't) and what it expects to see.

When it sees words that it doesn't recognize and doesn't expect, it will complain (in the form of a 💬 and an ❌ ). The comment should be enough to enable contributors to identify the items they've misspelled (and fix them), or files that they should exclude (and how to do that), or hint at how they could add a pattern in order to ignore pieces of lines.

In general, I'll watch a repository for at least a month in order to ensure smooth adoption. I'll always try to respond to @.

3. Does this change impact docs?

  • Yes, PR includes docs updates
  • Yes, issue opened: link to issue
  • No

If necessary, changes could be made to CONTRIBUTING.md#contributing via pull requests, but I don't think that's necessary. In general the workflow should be sufficiently user friendly for contributors to understand what they need to do to make it happy.

I'd encourage testing the workflow in a fork (your own, or feel free to use check-spelling/karpenter. Create commits/pull requests with typos and push them to GitHub to see how it behaves.

Note:
You will want to enable branch protection:
image

Otherwise you risk merging a typo and confusing other contributors when checks run for their PR.

The workflow, as configured, when run for a Pull Request checks the code as it would look if merged as opposed to simply checking the branch as it is. This is important since configuration changes (.github/actions/spelling) affect not just the content of the branch but any additional content that may have been added (or removed) in the destination branch.

Sample output

For reference, here's a run that I generated in tuning this PR:

Warning: hack/feature_request_reactions.py: line 5, columns 8-10, Warning - sys is not a recognized word. (unrecognized-spelling)
Warning: hack/feature_request_reactions.py: line 6, columns 22-31, Warning - itemgetter is not a recognized word. (unrecognized-spelling)
Warning: website/content/main.py: line 16, columns 16-22, Warning - r'https is not a recognized word. (unrecognized-spelling)
Warning: hack/feature_request_reactions.py: line 43, columns 47-56, Warning - itemgetter is not a recognized word. (unrecognized-spelling)
Warning: hack/feature_request_reactions.py: line 52, columns 21-23, Warning - sys is not a recognized word. (unrecognized-spelling)
Warning: hack/label_issue_count.py: line 5, columns 8-10, Warning - sys is not a recognized word. (unrecognized-spelling)
Warning: hack/label_issue_count.py: line 36, columns 21-23, Warning - sys is not a recognized word. (unrecognized-spelling)

In this case, the items it identified are acceptable, so I added them to expect.txt:
https://github.com/check-spelling/karpenter/compare/81486f4c63f5e4ee8a0fa653f223d1d59e687cae..a3830d6e09fe03c7ae8397ec485f1dcf95b9b831
The extra pattern addresses r'https which comes from some Python. It should be good enough to cover any of the various Python formats. The patterns file includes a link to a list of samples which should help if people need to write additional patterns.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@netlify
Copy link

netlify bot commented Dec 30, 2021

✔️ Deploy Preview for karpenter-docs-prod canceled.

🔨 Explore the source changes: a3830d6

🔍 Inspect the deploy log: https://app.netlify.com/sites/karpenter-docs-prod/deploys/61cd22dfd0818a0007dda6af

@ellistarn
Copy link
Contributor

Cool tool, thank you! I'll cut a PR on check-spelling with some feature requests.

@ellistarn ellistarn merged commit 77017ac into aws:main Dec 30, 2021
@jsoref jsoref deleted the spell-check branch December 30, 2021 19:47
ellistarn added a commit to ellistarn/karpenter-provider-aws that referenced this pull request Dec 31, 2021
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