8000 Add codespell test by Sheetalpamecha · Pull Request #2089 · RamenDR/ramen · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Sheetalpamecha
Copy link
Contributor
@Sheetalpamecha Sheetalpamecha commented Jun 12, 2025

Fixes: #2023

Changes -

  • github action workflow will run on every pull request
  • including make codespell to check for errors locally

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>
@Sheetalpamecha
Copy link
Contributor Author

Tested on local repo with codespell failing on the PR -
Screenshot 2025-06-05 at 12 47 22 PM

In 2nd ss None of the modified files require changes and codespell Passing -
Screenshot 2025-06-05 at 7 06 55 PM

Further, will add a new commit in this PR to handle all existing differences.

Copy link
Member
@nirs nirs left a 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
Copy link
Member

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:
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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.

Copy link
Member
@ShyamsundarR ShyamsundarR left a 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:
Copy link
Member

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
Copy link
Member

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?

Copy link
Member

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

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.

Add codespell test
3 participants
0