-
Notifications
You must be signed in to change notification settings - Fork 63
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
feat: GitHub workflow Codespell #256
feat: GitHub workflow Codespell #256
Conversation
Signed-off-by: Vara Bonthu <[email protected]>
Signed-off-by: Vara Bonthu <[email protected]>
Signed-off-by: Vara Bonthu <[email protected]>
@@ -11,7 +11,7 @@ import ( | |||
"sigs.k8s.io/controller-runtime/pkg/log" | |||
) | |||
|
|||
type Conainer struct { | |||
type Container struct { |
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 can revert this if this is going to cause an issue
@@ -59,7 +59,7 @@ func (f *FinchRuntime) ContainerWithPort(ctx context.Context, name string, port | |||
return false, err | |||
} | |||
|
|||
var containers []Conainer | |||
var containers []Container |
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.
same as above
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.
Looks like these are all contained in this file only so I don't expect to run into any issues.
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 think it's nice that this catches and fixes typos (obviously we have a lot). Is it possible to exclude this check when determining if the PR passes overall checks?
.pre-commit-config.yaml
Outdated
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.
Is this necessary? I just don't want to introduce another thing to manage.
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.
This setup is useful for contributors to run pre-commit
locally before raising a PR to check for any spelling errors. We don't need to maintain this file as it doesn't require any changes.
Signed-off-by: Vara Bonthu <[email protected]>
PR checks won't fail with the latest changes |
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 have a lot of typos as shown by this PR and I think it's a good idea to have this. We should have this in the stacks repository as well. Especially when we move the examples directory to it.
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.
LGTM
Feature: Implement Codespell for Automatic Spell Checking
This pull request introduces a new GitHub workflow that leverages the Codespell tool to automatically check for spelling errors across our codebase.
Benefits:
How to Use:
pre-commit run -a
from the root folder. You may have to install pre-commit locally