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

run linters on macOS and Windows as well #38

Merged
merged 1 commit into from
Mar 31, 2021

Conversation

thaJeztah
Copy link
Member

Mostly to catch if we correctly specified build-tags (see #36)

@thaJeztah
Copy link
Member Author

This will fail currently (until #36 is merged; will rebase after to verify it fixes that in CI)

@thaJeztah thaJeztah force-pushed the lint_other_platforms branch from 70c79cb to fcb04e2 Compare March 25, 2021 15:15
@thaJeztah thaJeztah marked this pull request as ready for review March 25, 2021 15:15
@thaJeztah
Copy link
Member Author

Arg... more compile errors on Windows;

level=error msg="[linters context] typechecking error: D:\\a\\fifo\\fifo\\src\\github.com\\containerd\\fifo\\handle_nolinux.go:34:19: Stat_t not declared by package syscall"
  level=error msg="[linters context] typechecking error: D:\\a\\fifo\\fifo\\src\\github.com\\containerd\\fifo\\handle_nolinux.go:35:20: Stat not declared by package syscall"
  level=error msg="[linters context] typechecking error: D:\\a\\fifo\\fifo\\src\\github.com\\containerd\\fifo\\mkfifo_nosolaris.go:24:17: Mkfifo not declared by package syscall"
  level=error msg="[linters context] typechecking error: D:\\a\\fifo\\fifo\\src\\github.com\\containerd\\fifo\\raw_test.go:220:27: cannot use int(fd) (value of type int) as syscall.Handle value in argument to syscall.Write"
  level=error msg="[linters context] typechecking error: D:\\a\\fifo\\fifo\\src\\github.com\\containerd\\fifo\\raw_test.go:240:26: cannot use int(fd) (value of type int) as syscall.Handle value in argument to syscall.Read"
  level=warning msg="[runner] Can't run linter goanalysis_metalinter: S1037: failed prerequisites: [([email protected]/containerd/fifo [github.com/containerd/fifo.test], [email protected]/containerd/fifo [

@thaJeztah thaJeztah force-pushed the lint_other_platforms branch 8 times, most recently from d5bee1f to f6abba5 Compare March 25, 2021 17:05
@thaJeztah
Copy link
Member Author

golangci-lint.. I hate you.. this is absolutely useless; somewhere, some place a file is detected as "not gofmt'ed".. so.. where????

  Running [D:\a\_temp\25b45d69-8406-47bb-b2cb-5dda8cdb0a43\golangci-lint-1.29.0-windows-amd64\golangci-lint run --out-format=github-actions --path-prefix=src/github.com/containerd/fifo] in [D:\a\fifo\fifo\src\github.com\containerd\fifo] ...
  Error: File is not `gofmt`-ed with `-s` (gofmt)
  Error: File is not `gofmt`-ed with `-s` (gofmt)
  
  Error: issues found
  Ran golangci-lint in 16413ms

@mxpv
Copy link
Member

mxpv commented Mar 28, 2021

This might be because of line endings.
go expects unix line endings on all platforms (golang/go#16355).
while github actions git setup does CRLF conversions by default, making golangci-lint to complain (golangci/golangci-lint#580).
I did the following fix in containerd repo: containerd/containerd@8b03df2

@thaJeztah
Copy link
Member Author

Oh! Thanks! Let me give that a try

@thaJeztah thaJeztah force-pushed the lint_other_platforms branch 3 times, most recently from 23fff05 to a178792 Compare March 29, 2021 12:13
@thaJeztah
Copy link
Member Author

That was it! You're a hero! It's green now (with #39 included, so still as "draft")

Mostly to catch if we correctly specified build-tags

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah thaJeztah force-pushed the lint_other_platforms branch from a178792 to a0bdb4a Compare March 30, 2021 13:54
@thaJeztah thaJeztah marked this pull request as ready for review March 30, 2021 13:54
@thaJeztah
Copy link
Member Author

#39 was merged; rebased and moved this out of draft

@mxpv @estesp ptal

Copy link
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

@mxpv mxpv merged commit 650e8a8 into containerd:master Mar 31, 2021
@thaJeztah thaJeztah deleted the lint_other_platforms branch March 31, 2021 08:53
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.

3 participants