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.

Sign up for GitHub

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

Jump to bottom

improve repo tooling #805

Merged
merged 6 commits into from
Aug 23, 2019
Merged

improve repo tooling #805

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6360dd0
nit shell scripts
BenTheElder Aug 22, 2019
d8d9dc1
cleanup go_container.sh, upgrade to go1.13rc1
BenTheElder Aug 23, 2019
e2bfd03
switch to golangci-lint
BenTheElder Aug 23, 2019
ca3a118
fix errcheck errors
BenTheElder Aug 23, 2019
245ca10
fix spelling errors
BenTheElder Aug 23, 2019
a116f7f
cleanup comments
BenTheElder Aug 23, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 22 additions & 3 deletions hack/tools/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,31 @@ module sigs.k8s.io/kind/hack/tools
go 1.12

require (
github.com/client9/misspell v0.3.4
github.com/golangci/golangci-lint v1.17.2-0.20190714145355-d2b1eea2c617
github.com/spf13/pflag v1.0.3 // indirect
golang.org/x/lint v0.0.0-20190409202823-959b441ac422
golang.org/x/crypto v0.0.0-20190510104115-cbcb75029529 // indirect
golang.org/x/tools v0.0.0-20190606174628-0139d5756a7d // indirect
honnef.co/go/tools v0.0.0-20190605142022-0a11fc526260
k8s.io/code-generator v0.0.0-20190311093542-50b561225d70
k8s.io/gengo v0.0.0-20190327210449-e17681d19d3a // indirect
k8s.io/klog v0.3.0 // indirect
)

// deal with golangci-lint being broken
Copy link
Member Author

Choose a reason for hiding this comment

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

golangci/golangci-lint#595 (comment)

Copy link
Member Author

Choose a reason for hiding this comment

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

basically: go 1.13 is stricter about the timestamps matching the source when using a pseudo-version, the module proxy is equally strict now even in 1.12 / 1.11. golangci-lint needs to update their go.mod upstream, there are PRs open for this.


replace github.com/golangci/errcheck v0.0.0-20181003203344-ef45e06d44b6 => github.com/golangci/errcheck v0.0.0-20181223084120-ef45e06d44b6

replace github.com/golangci/go-tools v0.0.0-20180109140146-af6baa5dc196 => github.com/golangci/go-tools v0.0.0-20190318060251-af6baa5dc196

replace github.com/golangci/gofmt v0.0.0-20181105071733-0b8337e80d98 => github.com/golangci/gofmt v0.0.0-20181222123516-0b8337e80d98

replace github.com/golangci/gosec v0.0.0-20180901114220-66fb7fc33547 => github.com/golangci/gosec v0.0.0-20190211064107-66fb7fc33547

replace github.com/golangci/lint-1 v0.0.0-20180610141402-ee948d087217 => github.com/golangci/lint-1 v0.0.0-20190420132249-ee948d087217

replace github.com/go-critic/go-critic v0.0.0-20181204210945-1df300866540 => github.com/go-critic/go-critic v0.0.0-20190526074819-1df300866540

replace mvdan.cc/unparam v0.0.0-20190124213536-fbb59629db34 => mvdan.cc/unparam v0.0.0-20190209190245-fbb59629db34

replace github.com/golangci/ineffassign v0.0.0-20180808204949-42439a7714cc => github.com/golangci/ineffassign v0.0.0-20190609212857-42439a7714cc

replace github.com/timakin/bodyclose => github.com/golangci/bodyclose v0.0.0-20190714144026-65da19158fa2
219 changes: 208 additions & 11 deletions hack/tools/go.sum
View file
Open in desktop

Large diffs are not rendered by default.

4 changes: 1 addition & 3 deletions hack/tools/tools.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@ package tools

import (
// linter(s)
_ "github.com/client9/misspell"
_ "golang.org/x/lint"
_ "honnef.co/go/tools/cmd/staticcheck"
_ "github.com/golangci/golangci-lint/cmd/golangci-lint"

// kubernetes code generators
_ "k8s.io/code-generator/cmd/conversion-gen"
Expand Down
36 changes: 3 additions & 33 deletions hack/verify/all.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,39 +24,9 @@ res=0

# run all verify scripts, optionally skipping any of them

if [[ "${VERIFY_SHELLCHECK:-true}" == "true" ]]; then
echo "verifying shellcheck ..."
hack/verify/shellcheck.sh || res=1
cd "${REPO_ROOT}"
fi

if [[ "${VERIFY_SPELLING:-true}" == "true" ]]; then
Copy link
Member Author

Choose a reason for hiding this comment

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

these are all in lint.sh now, by way of golangci-lint

echo "verifying spelling ..."
hack/verify/spelling.sh || res=1
cd "${REPO_ROOT}"
fi

if [[ "${VERIFY_GOFMT:-true}" == "true" ]]; then
echo "verifying gofmt ..."
hack/verify/gofmt.sh || res=1
cd "${REPO_ROOT}"
fi

if [[ "${VERIFY_GOLINT:-true}" == "true" ]]; then
echo "verifying golint ..."
hack/verify/golint.sh || res=1
cd "${REPO_ROOT}"
fi

if [[ "${VERIFY_GOVET:-true}" == "true" ]]; then
echo "verifying govet ..."
hack/verify/govet.sh || res=1
cd "${REPO_ROOT}"
fi

if [[ "${VERIFY_STATICCHECK:-true}" == "true" ]]; then
echo "verifying staticcheck ..."
hack/verify/staticcheck.sh || res=1
if [[ "${VERIFY_LINT:-true}" == "true" ]]; then
echo "verifying lints ..."
hack/verify/lint.sh || res=1
cd "${REPO_ROOT}"
fi

Expand Down
29 changes: 0 additions & 29 deletions hack/verify/gofmt.sh
View file
Open in desktop

This file was deleted.

36 changes: 0 additions & 36 deletions hack/verify/golint.sh
View file
Open in desktop

This file was deleted.

28 changes: 0 additions & 28 deletions hack/verify/govet.sh
View file
Open in desktop

This file was deleted.

23 changes: 9 additions & 14 deletions hack/verify/staticcheck.sh → hack/verify/lint.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,24 +13,19 @@
# See the License for the specific language governing permissions and
# limitations under the License.

# CI script to run staticcheck
# script to run linters
set -o errexit -o nounset -o pipefail

# cd to the repo root
REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd -P)"
cd "${REPO_ROOT}"

# enable modules and the proxy cache
export GO111MODULE="on"
GOPROXY="${GOPROXY:-https://proxy.golang.org}"
export GOPROXY
# build golangci-lint
SOURCE_DIR="${REPO_ROOT}/hack/tools" hack/go_container.sh \
go build -o /out/golangci-lint github.com/golangci/golangci-lint/cmd/golangci-lint

# build staticcheck
BINDIR="${REPO_ROOT}/bin"
# use the tools module
cd "hack/tools"
go build -o "${BINDIR}/staticcheck" honnef.co/go/tools/cmd/staticcheck
# go back to the root
cd "${REPO_ROOT}"

"${BINDIR}/staticcheck" ./pkg/... ./cmd/... .
# run golangci-lint
GO111MODULE=on bin/golangci-lint \
--enable=golint --enable=vet --enable=gofmt \
--enable=misspell \
run ./pkg/... ./cmd/... .
Copy link
Member Author

Choose a reason for hiding this comment

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

this includes staticcheck and other lints by default, we're explicitly enabling a few that are not on by default:

  • standard go lints (golint, vet, gofmt)
  • misspell

Copy link
Member Author

Choose a reason for hiding this comment

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

iterative (hot) run:

[bentheelder@bentheelder-macbookpro:~/go/src/sigs.k8s.io/kind·2019-08-22T21:38:14-0700·shell-nits@245ca10]
$ time hack/verify/lint.sh 

real    0m7.842s
user    0m23.441s
sys     0m4.258s

much faster than running these individually.

I'm exploring the additional lints we can enable, but may leave that for a follow up.

Copy link
Contributor

Choose a reason for hiding this comment

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

that's progress

Copy link
Member Author

Choose a reason for hiding this comment

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

it's more like a minute on a cold run, but that's not bad considering how much static analysis we're getting. this already caught some bugs we hadn't before, just with the default checks.

56 changes: 0 additions & 56 deletions hack/verify/shellcheck.sh
View file
Open in desktop

This file was deleted.

37 changes: 0 additions & 37 deletions hack/verify/spelling.sh
View file
Open in desktop

This file was deleted.