Repository navigation
Conversation
|
Download PR build artifacts for linux_amd64.tar.gz, darwin_arm64.tar.gz, and windows_amd64.zip. 📦 Click here to get additional PR build artifactsArtifact URLs |
srebhan
left a comment
There was a problem hiding this comment.
Nice. Thanks @skartikey! Just one request...
| - run: | ||
| name: "golangci-lint/Windows arm64 and 386" | ||
| # Files with an architecture suffix (e.g. pdh_arm64.go) are not part of the amd64 run above, | ||
| # so lint the packages containing them for the other shipped Windows architectures | ||
| command: | | ||
| pkgs=$(git ls-files '*_arm64.go' '*_386.go' | xargs -n1 dirname | sort -u | sed 's|^|./|') | ||
| [ -n "$pkgs" ] || exit 0 | ||
| for arch in arm64 386; do | ||
| GOGC=80 GOMEMLIMIT=6144MiB GOOS=windows GOARCH=$arch $GOPATH/bin/golangci-lint run --verbose --timeout=30m --concurrency 4 $pkgs | ||
| done | ||
| no_output_timeout: 30m |
There was a problem hiding this comment.
Can we please split this into two steps!?
There was a problem hiding this comment.
Done, one step per architecture now, each deriving its own package list.
srebhan
left a comment
There was a problem hiding this comment.
Thanks @skartikey! My only concern is that not all files do adhere to the naming convention you use for filtering.
| - run: | ||
| name: "golangci-lint/Windows arm64" | ||
| # Files with an architecture suffix (e.g. pdh_arm64.go) are not part of the amd64 run above, | ||
| # so lint the packages containing them for that architecture | ||
| command: | | ||
| pkgs=$(git ls-files '*_arm64.go' | xargs -n1 dirname | sort -u | sed 's|^|./|') | ||
| [ -n "$pkgs" ] || exit 0 | ||
| GOGC=80 GOMEMLIMIT=6144MiB GOOS=windows GOARCH=arm64 $GOPATH/bin/golangci-lint run --verbose --timeout=30m --concurrency 4 $pkgs | ||
| no_output_timeout: 30m |
There was a problem hiding this comment.
The problem I see here is that not all arch-specific files adhere to this convention. Do you think it would make sense to lint the whole project? I guess it will prolong the whole pipeline, won't it?
Summary
The
lint-windowsjob runs with the executor'sGOARCH=amd64, so files with an architecture suffix likepdh_arm64.goandpdh_386.goare never linted, even though we ship Windows arm64 and i386 builds. This adds one step per architecture to the same job, linting the packages containing*_arm64.gofiles for arm64 and*_386.gofiles for 386. The lists are derived from the file names, so new architecture-specific files are covered automatically, and only that one package is linted today rather than the whole repository.It also clears the findings that were hiding there: comment spacing in both files, and an unused
pdhFmtCountervalueLongtype inpdh_386.gowhose//nolintsat on the wrong line. The amd64 variant does not have that type, so it is removed rather than suppressed.Checklist
Related issues
resolves #19773