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-checks,.golangci.yml: move checks from run-checks to lints #14721

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 15 additions & 4 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,20 @@ linters-settings:
deny:
- pkg: "os/user"
desc: "Please use osutil/user instead. See https://github.com/canonical/snapd/pull/13776"
mathrand:
files:
- "!**/randutil/*.go"
- "!**/vendor/**"
- "!$test"
deny:
- pkg: "math/rand"
desc: "Direct usage of math/rand, we prefer randutil"
ioioutil:
files:
- "!**/vendor/**"
deny:
- pkg: "io/ioutil"
desc: "Found usage of deprecated io/ioutil, please use \"io\" or \"os\" equivalents"

misspell:
# Correct spellings using locale preferences for US or UK.
Expand All @@ -140,8 +154,6 @@ linters-settings:
- auther
- PROCES
- PROCESSS
- proces
- processs
- exportfs
lll:
# max line length, lines longer will be reported. Default is 120.
Expand Down Expand Up @@ -183,8 +195,7 @@ linters:
# gosimple may suggest patterns that work only with more recent Go versions
# - gosimple
- nakedret
# formatting is disabled until we move to Go 1.13
# - gofmt
- gofmt
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this nullify the need for separate fmt checks in run-checks?

Copy link
Contributor

Choose a reason for hiding this comment

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

let's double check, there's likely more linters we have in run-checks that may not be needed anymore

Copy link
Member Author

Choose a reason for hiding this comment

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

The manual one in run-checks runs with -s -d, which prints out a nice diff, whereas the one in golangci-lint only supports -s. So the former is what we're used to and is nicer.

Regarding other lints, I did check, and some of the others in run-checks apply to all files, while the golangci-lint checks only seem to apply to .go files. So technically I think we want to preserve the superset of checks in run-checks in some cases.

I don't think we need nakedret in run-checks anymore, though.

- ineffassign
# disabling until https://github.com/daixiang0/gci/issues/54 is fixed
# - gci
Expand Down
34 changes: 1 addition & 33 deletions run-checks
Original file line number Diff line number Diff line change
Expand Up @@ -209,38 +209,6 @@ if [ "$STATIC" = 1 ]; then
exit 1
fi

echo "Checking for direct usages of math/rand"
got=""
for dir in $(go list -f '{{.Dir}}' ./... | grep -v '/vendor/' ); do
# shellcheck disable=SC2063
s="$(grep -nP --exclude '*_test.go' --exclude 'randutil/*.go' math/rand "$dir"/*.go || true)"
if [ -n "$s" ]; then
got="$s\\n$got"
fi
done

if [ -n "$got" ]; then
echo 'Direct usages of math/rand, we prefer randutil:'
echo "$got"
exit 1
fi

echo "Checking for usages of deprecated io/ioutil"
got=""
for dir in $(go list -f '{{.Dir}}' ./... | grep -v '/vendor/' ); do
# shellcheck disable=SC2063
s="$(grep -nP io/ioutil "$dir"/*.go || true)"
if [ -n "$s" ]; then
got="$s\\n$got"
fi
done

if [ -n "$got" ]; then
echo 'Found usages of deprecated io/ioutil, please use "io" or "os" equivalents'
echo "$got"
exit 1
fi

if command -v shellcheck >/dev/null; then
exclude_tools_path=tests/lib/external/snapd-testing-tools
echo "Checking shell scripts..."
Expand Down Expand Up @@ -304,7 +272,7 @@ if [ "$STATIC" = 1 ]; then
# PROCES is used in the seccomp tests (PRIO_PROCES{,S,SS})
# exportfs is used in the nfs-support test
# becuase because of misspell in upstream steam rules (PR#12657)
MISSPELL_IGNORE="auther,PROCES,PROCESSS,proces,processs,exportfs,becuase"
MISSPELL_IGNORE="auther,PROCES,PROCESSS,exportfs,becuase"
git ls-files -z -- . ':!:./po' ':!:./vendor' ':!:./c-vendor' ':!:./cmd/libsnap-confine-private/bpf/vendor' ':!:./build-aux/snap/local/apparmor'|
xargs -0 misspell -error -i "$MISSPELL_IGNORE"
fi
Expand Down
Loading