Skip to content

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