Skip to content
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

CI: add more linters #92

Merged
merged 6 commits into from Nov 4, 2021
Merged

CI: add more linters #92

merged 6 commits into from Nov 4, 2021

Conversation

kolyshkin
Copy link
Collaborator

@kolyshkin kolyshkin commented Oct 28, 2021

(currently a draft pending #90 merge)

  • Enable some linters, fix their warnings.
  • Update golangci-lint to 1.43.0.
  • Force LF line endings for .go files.

Fixes: #91

@thaJeztah
Copy link
Member

@kolyshkin can you rebase this one and move it out of draft?

@thaJeztah
Copy link
Member

(changes look good)

This warning

> sharedsubtree_linux_test.go:329:74: comparing with != will fail on wrapped errors. Use errors.Is to check for a specific error (errorlint)
> 	if err := Mount(sourceDir, targetDir, "none", "bind,rw"); err != nil && errors.Unwrap(err) != unix.EINVAL {
>	                                                                        ^

is not making much sense, since the code is legit, but in general it's
better to use errors.Is, so let's switch to it.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Errors returned from x/sys/unix are bare, so it's OK to compare
directly.

Fixes warnings like this one:

> mount_unix.go:26:19: comparing with == will fail on wrapped errors. Use errors.Is to check for a specific error (errorlint)
> 	if err == nil || err == unix.EINVAL {
> 	                 ^

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

This presumably fails on windows because of windows-style line endings, presumably brought in by git checkout.

Looking.

Force UNIX-style line endings for go files.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Release notes: https://github.com/golangci/golangci-lint/releases

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

This presumably fails on windows because of windows-style line endings, presumably brought in by git checkout.

Yes. Basically, git converts text files (including *.go) to use CRLF (aka Windows-style) line endings, and then gofumpt complains about bad formatting (in a not-so-clear way).

Apparently, in Go every file is supposed to have LF (aka UNIX-style) line endings even on Windows (see e.g. golang/go#16355), this way the formatting is truly enforced. The way to do that wrt git is to have .gitattributes file similar to the one this PR adds.

@thaJeztah
Copy link
Member

CRLF should be banned! 😂

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

@thaJeztah thaJeztah merged commit afb7f50 into moby:master Nov 4, 2021
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.

CI: enable more linters
2 participants