Skip to content

Fix codespell warnings, add codespell to ci#3144

Merged
AkihiroSuda merged 1 commit intoopencontainers:masterfrom
kolyshkin:codespell
Aug 18, 2021
Merged

Fix codespell warnings, add codespell to ci#3144
AkihiroSuda merged 1 commit intoopencontainers:masterfrom
kolyshkin:codespell

Conversation

@kolyshkin
Copy link
Copy Markdown
Contributor

@kolyshkin kolyshkin commented Aug 11, 2021

Fix a bunch of warnings from codespell, and add a CI job to validate to automate it.

Note that

  1. I had to use pip to install codespell, as the version provided with Ubuntu 20.04 is way old.
  2. The two exceptions I had to add to codespellrc are:
  • CLOS (a term used by intelrtd);
  • creat (syscall name used in tests/integration/testdata/seccomp_*.json).

@kolyshkin

This comment has been minimized.

AkihiroSuda
AkihiroSuda previously approved these changes Aug 11, 2021
@kolyshkin kolyshkin requested a review from thaJeztah August 16, 2021 23:23
# must be specified without patch version
version: v1.40

codespell:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious; have you also tried enabling misspell in golang-ci-lint? Mostly wondering if it would catch the same errors (it would save us from having to run a separate linter for this); https://golangci-lint.run/usage/linters

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I guess misspell may only be looking for .go files 🤔

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's right; as part of golangci-lint it will probably only check .go files.

It is also

  • apparently unmaintained since March 2018 (judging by its git repo at https://github.com/client9/misspell)
  • can only find two misspellings in the very same codebase as this PR, when run as a standalone binary (checks all files);
[kir@kir-rhat ~]$ go1.17 install github.com/client9/misspell/cmd/misspell@latest

[kir@kir-rhat runc]$ misspell * | grep -v ^vendor/
libcontainer/cgroups/ebpf/ebpf_linux.go:137:23: "succeded" is a misspelling of "succeeded"
libcontainer/cgroups/systemd/systemd_test.go:344:22: "contianer" is a misspelling of "container"

(the last typo it found is the one codespell can't see, but I've added the fix to this PR)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addition to the above, there's something that makes me think you'd favor codespell over it 🙃

Once I've rebased:

[kir@kir-rhat runc]$ misspell * | grep -v ^vendor/
MAINTAINERS:8:0: "Sebastiaan" is a misspelling of "Sebastian"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LOL! 😂

The two exceptions I had to add to codespellrc are:
 - CLOS (used by intelrtd);
 - creat (syscall name used in tests/integration/testdata/seccomp_*.json).

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@kolyshkin
Copy link
Copy Markdown
Contributor Author

Rebased to resolve a conflict after #3164 merge; PTAL @thaJeztah @AkihiroSuda

Copy link
Copy Markdown
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@AkihiroSuda AkihiroSuda merged commit 4d26c4a into opencontainers:master Aug 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants