Skip to content

Commit

Permalink
Moves lint and format to make, managed by bingo
Browse files Browse the repository at this point in the history
This removes a few shell scripts and in the process updates linters to
more current versions. For example, we were using golangci from 2019. To
adjust, some invalid statements were removed.

The main part of this change is using https://github.com/bwplotka/bingo
as the linters we used were go programs anyway. Bingo generates make
entries you can use to on-demang install tools without affecting your
go.mod and without shell scripts.

Signed-off-by: Adrian Cole <adrian@tetrate.io>
  • Loading branch information
Adrian Cole committed Mar 25, 2021
1 parent 5c46136 commit dd0b482
Show file tree
Hide file tree
Showing 124 changed files with 221 additions and 327 deletions.
12 changes: 12 additions & 0 deletions .bingo/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@

# Ignore everything
*

# But not these files:
!.gitignore
!*.mod
!README.md
!Variables.mk
!variables.env

*tmp.mod
14 changes: 14 additions & 0 deletions .bingo/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# Project Development Dependencies.

This is directory which stores Go modules with pinned buildable package that is used within this repository, managed by https://github.com/bwplotka/bingo.

* Run `bingo get` to install all tools having each own module file in this directory.
* Run `bingo get <tool>` to install <tool> that have own module file in this directory.
* For Makefile: Make sure to put `include .bingo/Variables.mk` in your Makefile, then use $(<upper case tool name>) variable where <tool> is the .bingo/<tool>.mod.
* For shell: Run `source .bingo/variables.env` to source all environment variable for each tool.
* For go: Import `.bingo/variables.go` to for variable names.
* See https://github.com/bwplotka/bingo or -h on how to add, remove or change binaries dependencies.

## Requirements

* Go 1.14+
43 changes: 43 additions & 0 deletions .bingo/Variables.mk
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
# Auto generated binary variables helper managed by https://github.com/bwplotka/bingo v0.4.0. DO NOT EDIT.
# All tools are designed to be build inside $GOBIN.
BINGO_DIR := $(dir $(lastword $(MAKEFILE_LIST)))
GOPATH ?= $(shell go env GOPATH)
GOBIN ?= $(firstword $(subst :, ,${GOPATH}))/bin
GO ?= $(shell which go)

# Below generated variables ensure that every time a tool under each variable is invoked, the correct version
# will be used; reinstalling only if needed.
# For example for goimports variable:
#
# In your main Makefile (for non array binaries):
#
#include .bingo/Variables.mk # Assuming -dir was set to .bingo .
#
#command: $(GOIMPORTS)
# @echo "Running goimports"
# @$(GOIMPORTS) <flags/args..>
#
GOIMPORTS := $(GOBIN)/goimports-v0.1.0
$(GOIMPORTS): $(BINGO_DIR)/goimports.mod
@# Install binary/ries using Go 1.14+ build command. This is using bwplotka/bingo-controlled, separate go module with pinned dependencies.
@echo "(re)installing $(GOBIN)/goimports-v0.1.0"
@cd $(BINGO_DIR) && $(GO) build -mod=mod -modfile=goimports.mod -o=$(GOBIN)/goimports-v0.1.0 "golang.org/x/tools/cmd/goimports"

GOLANGCI_LINT := $(GOBIN)/golangci-lint-v1.38.0
$(GOLANGCI_LINT): $(BINGO_DIR)/golangci-lint.mod
@# Install binary/ries using Go 1.14+ build command. This is using bwplotka/bingo-controlled, separate go module with pinned dependencies.
@echo "(re)installing $(GOBIN)/golangci-lint-v1.38.0"
@cd $(BINGO_DIR) && $(GO) build -mod=mod -modfile=golangci-lint.mod -o=$(GOBIN)/golangci-lint-v1.38.0 "github.com/golangci/golangci-lint/cmd/golangci-lint"

LICENSER := $(GOBIN)/licenser-v0.6.0
$(LICENSER): $(BINGO_DIR)/licenser.mod
@# Install binary/ries using Go 1.14+ build command. This is using bwplotka/bingo-controlled, separate go module with pinned dependencies.
@echo "(re)installing $(GOBIN)/licenser-v0.6.0"
@cd $(BINGO_DIR) && $(GO) build -mod=mod -modfile=licenser.mod -o=$(GOBIN)/licenser-v0.6.0 "github.com/liamawhite/licenser"

SHFMT := $(GOBIN)/shfmt-v3.2.4
$(SHFMT): $(BINGO_DIR)/shfmt.mod
@# Install binary/ries using Go 1.14+ build command. This is using bwplotka/bingo-controlled, separate go module with pinned dependencies.
@echo "(re)installing $(GOBIN)/shfmt-v3.2.4"
@cd $(BINGO_DIR) && $(GO) build -mod=mod -modfile=shfmt.mod -o=$(GOBIN)/shfmt-v3.2.4 "mvdan.cc/sh/v3/cmd/shfmt"

1 change: 1 addition & 0 deletions .bingo/go.mod
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module _ // Fake go.mod auto-created by 'bingo' for go -moddir compatibility with non-Go projects. Commit this file, together with other .mod files.
5 changes: 5 additions & 0 deletions .bingo/goimports.mod
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module _ // Auto generated by https://github.com/bwplotka/bingo. DO NOT EDIT

go 1.16

require golang.org/x/tools v0.1.0 // cmd/goimports
5 changes: 5 additions & 0 deletions .bingo/golangci-lint.mod
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module _ // Auto generated by https://github.com/bwplotka/bingo. DO NOT EDIT

go 1.16

require github.com/golangci/golangci-lint v1.38.0 // cmd/golangci-lint
5 changes: 5 additions & 0 deletions .bingo/licenser.mod
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module _ // Auto generated by https://github.com/bwplotka/bingo. DO NOT EDIT

go 1.16

require github.com/liamawhite/licenser v0.6.0
5 changes: 5 additions & 0 deletions .bingo/shfmt.mod
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module _ // Auto generated by https://github.com/bwplotka/bingo. DO NOT EDIT

go 1.16

require mvdan.cc/sh/v3 v3.2.4 // cmd/shfmt
18 changes: 18 additions & 0 deletions .bingo/variables.env
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Auto generated binary variables helper managed by https://github.com/bwplotka/bingo v0.4.0. DO NOT EDIT.
# All tools are designed to be build inside $GOBIN.
# Those variables will work only until 'bingo get' was invoked, or if tools were installed via Makefile's Variables.mk.
GOBIN=${GOBIN:=$(go env GOBIN)}

if [ -z "$GOBIN" ]; then
GOBIN="$(go env GOPATH)/bin"
fi


GOIMPORTS="${GOBIN}/goimports-v0.1.0"

GOLANGCI_LINT="${GOBIN}/golangci-lint-v1.38.0"

LICENSER="${GOBIN}/licenser-v0.6.0"

SHFMT="${GOBIN}/shfmt-v3.2.4"

10 changes: 4 additions & 6 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,16 @@ jobs:
GOMAXPROCS: "3"
steps:
- checkout
- run: make init
- run: ./ci/install-lint
- run: ./ci/lint

- run: make lint

test:
executor: builder
resource_class: medium+
steps:
- checkout
- run: make init
- run: ./ci/install-envoy
- run: ./ci/test
- run: go run cmd/getenvoy/main.go fetch standard:1.11.0
- run: make test.ci GO_TEST_EXTRA_OPTS="-timeout 60s"
- run:
name: "Measure test coverage (for now, on a subset of tests)"
command: make coverage GO_COVERAGE_EXTRA_OPTS="-p 1"
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ vendor
dist
/getenvoy
/build/
.idea

# code generated by `github.com/rakyll/statik`
statik.go
50 changes: 5 additions & 45 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@ run:
modules-download-mode: readonly

linters:
enable:
enable:
- "bodyclose"
# - "depguard" broken in golangci-lint 1.17.1
- "depguard"
- "dupl"
- "goconst"
- "gocritic"
Expand All @@ -46,17 +46,15 @@ linters:
- "golint"
- "gosec"
- "gosimple"
- "interfacer"
- "lll"
- "maligned"
- "misspell"
- "nakedret"
- "prealloc"
- "scopelint"
- "unconvert"
- "unparam"
disable:

disable:
- "gochecknoinits"
- "gochecknoglobals"

Expand Down Expand Up @@ -93,67 +91,30 @@ linters-settings:
unparam:
check-exported: false
gocritic:
enabled-checks:
- appendCombine
- appendAssign
- assignOp
- argOrder
- badCond
enabled-checks: # non-default checks we decided to also add
- boolExprSimplify
- builtinShadow
- captLocal
- caseOrder
- codegenComment
- commentedOutCode
- commentedOutImport
- defaultCaseOrder
- deprecatedComment
- docStub
- dupArg
- dupBranchBody
- dupCase
- dupSubExpr
- elseif
- emptyFallthrough
- equalFold
- flagDeref
- flagName
- hexLiteral
- indexAlloc
- initClause
- methodExprCall
- nilValReturn
- octalLiteral
- offBy1
- rangeExprCopy
- regexpMust
- singleCaseSwitch
- sloppyLen
- stringXbytes
- switchTrue
- typeAssertChain
- typeSwitchVar
- typeUnparen
- underef
- unlambda
- unnecessaryBlock
- unslice
- valSwap
- weakCond
- yodaStyleExpr
- commentFormatting
- emptyStringTest
- exitAfterDefer
- ifElseChain
- hugeParam
- importShadow
- nestingReduce
- paramTypeCombine
- ptrToRefParam
- rangeValCopy
- sloppyReassign
- unlabelStmt
- wrapperFunc

# Enable multiple checks by tags, run `GL_DEBUG=gocritic golangci-lint` run to see all tags and checks.
# Empty list by default. See https://github.com/go-critic/go-critic#usage -> section "Tags".
Expand All @@ -180,7 +141,6 @@ issues:
- errcheck
- dupl
- gosec
- maligned

# Exclude lll issues for long lines with go:generate
- linters:
Expand Down
4 changes: 4 additions & 0 deletions .licenserignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,7 @@ go.mod
go.sum

*.pem

.bingo
.dockerignore
.idea
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ We welcome contributions from the community. Please read the following guideline

## Coding Style

- The code is linted using a relatively stringent [golang-ci config](./.golangci.yml). To run this linter (and a few others) use the `ci/format` script. To verify, you can run the `ci/lint` script.
- The code is linted using a relatively stringent [golang-ci config](./.golangci.yml). To run this linter (and a few others) use run `make format`. To verify, you can run `make lint`.
- We follow standard Go table-driven tests and use the [`testify/assert`](https://github.com/stretchr/testify#assert-package) library to assert correctness. To verify all tests pass, you can run the `ci/test` script.

## DCO
Expand Down
35 changes: 35 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@
# See the License for the specific language governing permissions and
# limitations under the License.

# Make sure we pick up any local overrides.
-include .makerc

# bingo manages go binaries needed for building the project
include .bingo/Variables.mk

ENVOY = standard:1.11.1
HUB ?= docker.io/getenvoy
GETENVOY_TAG ?= dev
Expand Down Expand Up @@ -175,3 +181,32 @@ $(foreach lang,$(BUILDERS_LANGS),$(eval $(call GEN_PULL_EXTENSION_BUILDER_IMAGE_

.PHONY: builders.pull
builders.pull: $(foreach lang,$(BUILDERS_LANGS), pull/builder/$(lang))

##@ Code quality and integrity

LINT_OPTS ?= --timeout 5m
.PHONY: lint
# generate must be called while generated source is still used
lint: generate $(GOLANGCI_LINT) $(SHFMT) $(LICENSER) .golangci.yml ## Run the linters
@echo "--- lint ---"
@$(SHFMT) -d .
@$(LICENSER) verify -r .
@$(GOLANGCI_LINT) run $(LINT_OPTS) --config .golangci.yml

# The goimports tool does not arrange imports in 3 blocks if there are already more than three blocks.
# To avoid that, before running it, we collapse all imports in one block, then run the formatter.
.PHONY: format
format: $(GOIMPORTS) ## Format all Go code
@echo "--- format ---"
@$(LICENSER) apply -r "Tetrate"
@find . -type f -name '*.go' | xargs gofmt -s -w
@for f in `find . -name '*.go'`; do \
awk '/^import \($$/,/^\)$$/{if($$0=="")next}{print}' $$f > /tmp/fmt; \
mv /tmp/fmt $$f; \
$(GOIMPORTS) -w -local github.com/tetrateio/getenvoy $$f; \
done

.PHONY: clean
clean: ## Clean all binaries
@echo "--- $@ ---"
go clean -testcache
8 changes: 0 additions & 8 deletions ci/format

This file was deleted.

4 changes: 0 additions & 4 deletions ci/install-envoy

This file was deleted.

16 changes: 0 additions & 16 deletions ci/install-lint

This file was deleted.

28 changes: 0 additions & 28 deletions ci/lint

This file was deleted.

5 changes: 0 additions & 5 deletions ci/test

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"testing"

"github.com/stretchr/testify/require"

"github.com/tetratelabs/proxy-wasm-go-sdk/proxytest"
"github.com/tetratelabs/proxy-wasm-go-sdk/proxywasm/types"
)
Expand Down
Loading

0 comments on commit dd0b482

Please sign in to comment.