-
Notifications
You must be signed in to change notification settings - Fork 62
Add codespell test #2089
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
base: main
Are you sure you want to change the base?
Add codespell test #2089
Conversation
Changes - * github action workflow will run on every pull request * including make codespell to check for errors locally Signed-off-by: Sheetal Pamecha <spamecha@redhat.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Sheetalpamecha!
Please link the issue #2023 and check the issue example run for skipping files that cannot be checked.
@@ -0,0 +1,3 @@ | |||
[codespell] | |||
skip = *.svg |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to skip also go.sum, see ramenctl codespell job.
We need also other skips, see the spelling results in the issue. Many spelling errors should be skipped.
on: | ||
pull_request: | ||
|
||
jobs: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
job is missing permissions, see ramenctl for example.
python-version: '3.x' | ||
|
||
- name: Install codespell | ||
run: pip3 install codespell |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can use the codespell action, see ramenctl codespell job.
- name: Checkout repository | ||
uses: actions/checkout@v4 | ||
with: | ||
fetch-depth: 0 # Required to compare commits |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need to compare commits. Lets keep it simple.
git fetch origin ${{ github.base_ref }} | ||
git diff --name-only origin/${{ github.base_ref }}...HEAD \ | ||
| grep -vE '\.(svg)$' > changed-files.txt | ||
cat changed-files.txt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need this, we can check the entire project (after fixing the spelling issues).
# Add codespell target, Usage - make codespell | ||
.PHONY: codespell | ||
codespell: | ||
codespell --config .codespellrc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets call this "spell", and add also "spell-fix" - see ramenctl.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this check commit messages as well?
|
||
# Add codespell target, Usage - make codespell | ||
.PHONY: codespell | ||
codespell: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add this to make lint
as well please, so that changes are linted in one go. Possibly adding to pre-commit.sh for the tool requirement.
# SPDX-License-Identifier: Apache-2.0 | ||
|
||
--- | ||
name: Codespell Check |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need a separate workflow, maybe easier to add this to the linter phase in the ramen workflow itself?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would have a separate job so we get all errors in one run. If we add this to the lint job, we will not get spelling errors if lint fails, or not get lint errors if spelling fails. In ramenctl we have a separate job:
https://github.com/RamenDR/ramenctl/actions/runs/16049133721
Fixes: #2023
Changes -