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

Add go linting to CI config #2018

Merged
merged 3 commits into from
Dec 20, 2018
Merged
Show file tree
Hide file tree
Changes from 2 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
1 change: 1 addition & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ jobs:
- ./bin/protoc-go.sh
- go test -race -v ./...
- go vet ./...
- ./bin/lint

- language: node_js
node_js:
Expand Down
15 changes: 15 additions & 0 deletions Gopkg.lock
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,17 @@
pruneopts = ""
revision = "d9133f5469342136e669e85192a26056b587f503"

[[projects]]
branch = "master"
digest = "1:053024e8981dab276218a180e58f97034d06e8c4dd2e24c9365ad489104e1d90"
name = "golang.org/x/lint"
packages = [
".",
"golint",
]
pruneopts = ""
revision = "8f45f776aaf18cebc8d65861cc70c33c60471952"

[[projects]]
branch = "master"
digest = "1:e8c91565d4707bd93aa70e096884ce533acc5deb2bbb500bb064f49de70acda0"
Expand Down Expand Up @@ -525,6 +536,9 @@
name = "golang.org/x/tools"
packages = [
"go/ast/astutil",
"go/gcexportdata",
"go/internal/gcimporter",
"go/types/typeutil",
"imports",
"internal/fastwalk",
]
Expand Down Expand Up @@ -963,6 +977,7 @@
"github.com/sergi/go-diff/diffmatchpatch",
"github.com/sirupsen/logrus",
"github.com/spf13/cobra",
"golang.org/x/lint/golint",
"golang.org/x/net/context",
"google.golang.org/grpc",
"google.golang.org/grpc/codes",
Expand Down
1 change: 1 addition & 0 deletions Gopkg.toml
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
required = [
"github.com/golang/protobuf/protoc-gen-go",
"golang.org/x/lint/golint",
"k8s.io/code-generator/cmd/client-gen",
"k8s.io/code-generator/cmd/deepcopy-gen",
"k8s.io/code-generator/cmd/defaulter-gen",
Expand Down
23 changes: 23 additions & 0 deletions bin/lint
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
#!/bin/bash

set -eu

cd "$(pwd -P)"

bindir="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
rootdir="$( cd $bindir/.. && pwd )"

cd "$rootdir"

# install golint from vendor
go install ./vendor/golang.org/x/lint/golint

# use `go list` to exclude packages in vendor, ignore uncommented warnings
out=$(go list ./... | xargs golint | grep -v 'should have comment') || true
Copy link
Member

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

Choose a reason for hiding this comment

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

An alternative to this would be to use golangci-lint run --disable-all -E golint - this ignores all comment related linter errors, and in the future we could enable even more linters by adding a linter config file. If golint is the only one needed, then this is perfect.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wow! I had no idea that the go linting rabbit hole goes so deep :)

I wouldn't be opposed to adding additional linting configurations going forward, but I think golint minus comments suffices for our use case right now. That said, I'd happily review a PR that improved upon it. Thanks for the suggestion!


if [ -n "$out" ]; then
echo "$out"
exit 1
fi

echo "all clean!"
2 changes: 1 addition & 1 deletion cli/Dockerfile-bin
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
## compile binaries
FROM gcr.io/linkerd-io/go-deps:cd57a76a as golang
FROM gcr.io/linkerd-io/go-deps:4b31aa9b as golang
WORKDIR /go/src/github.com/linkerd/linkerd2
COPY cli cli
COPY controller/k8s controller/k8s
Expand Down
2 changes: 1 addition & 1 deletion controller/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
## compile controller services
FROM gcr.io/linkerd-io/go-deps:cd57a76a as golang
FROM gcr.io/linkerd-io/go-deps:4b31aa9b as golang
WORKDIR /go/src/github.com/linkerd/linkerd2
COPY controller/gen controller/gen
COPY pkg pkg
Expand Down
4 changes: 3 additions & 1 deletion controller/api/proxy/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ type server struct {
enableTLS bool
}

// The proxy-api service serves service discovery and other information to the
// NewServer returns a new instance of the proxy-api server.
//
// The proxy-api server serves service discovery and other information to the
// proxy. This implementation supports the "k8s" destination scheme and expects
// destination paths to be of the form:
// <service>.<namespace>.svc.cluster.local:<port>
Expand Down
1 change: 0 additions & 1 deletion controller/api/public/test_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,6 @@ type PodCounts struct {
FailedPods uint64
}

// satisfies v1.API
func (m *MockProm) Query(ctx context.Context, query string, ts time.Time) (model.Value, error) {
m.rwLock.Lock()
defer m.rwLock.Unlock()
Expand Down
4 changes: 2 additions & 2 deletions controller/api/util/api_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ var (
}
)

// Parameters that are used to build requests for metrics data. This includes
// requests to StatSummary and TopRoutes
// StatsBaseRequestParams contains parameters that are used to build requests
// for metrics data. This includes requests to StatSummary and TopRoutes
type StatsBaseRequestParams struct {
TimeWindow string
Namespace string
Expand Down
3 changes: 2 additions & 1 deletion controller/ca/ca.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"time"
)

// CA provides a certificate authority for TLS-enabled installs.
// Issuing certificates concurrently is not supported.
type CA struct {
// validity is the duration for which issued certificates are valid. This
Expand Down Expand Up @@ -104,7 +105,7 @@ func NewCA() (*CA, error) {
return &ca, nil
}

// TrustAnchorDER returns the PEM-encoded X.509 certificate of the trust anchor
// TrustAnchorPEM returns the PEM-encoded X.509 certificate of the trust anchor
// (root CA).
func (ca *CA) TrustAnchorPEM() string {
return ca.rootPEM
Expand Down
2 changes: 1 addition & 1 deletion controller/gen/apis/serviceprofile/v1alpha1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ type Range struct {

// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object

// MyResourceList is a list of MyResource resources
// ServiceProfileList is a list of ServiceProfile resources
type ServiceProfileList struct {
metav1.TypeMeta `json:",inline"`
metav1.ListMeta `json:"metadata"`
Expand Down
2 changes: 2 additions & 0 deletions pkg/util/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
httpPb "github.com/linkerd/linkerd2-proxy-api/go/http_types"
)

// ParseScheme converts a scheme string to protobuf
// TODO: validate scheme
func ParseScheme(scheme string) *httpPb.Scheme {
value, ok := httpPb.Scheme_Registered_value[strings.ToUpper(scheme)]
Expand All @@ -23,6 +24,7 @@ func ParseScheme(scheme string) *httpPb.Scheme {
}
}

// ParseMethod converts a method string to protobuf
// TODO: validate method
func ParseMethod(method string) *httpPb.HttpMethod {
value, ok := httpPb.HttpMethod_Registered_value[strings.ToUpper(method)]
Expand Down
2 changes: 1 addition & 1 deletion proxy-init/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
## compile proxy-init utility
FROM gcr.io/linkerd-io/go-deps:cd57a76a as golang
FROM gcr.io/linkerd-io/go-deps:4b31aa9b as golang
WORKDIR /go/src/github.com/linkerd/linkerd2
COPY ./proxy-init ./proxy-init
RUN CGO_ENABLED=0 GOOS=linux go install -v ./proxy-init/
Expand Down
2 changes: 1 addition & 1 deletion web/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ COPY web/app .
RUN $ROOT/bin/web build

## compile go server
FROM gcr.io/linkerd-io/go-deps:cd57a76a as golang
FROM gcr.io/linkerd-io/go-deps:4b31aa9b as golang
WORKDIR /go/src/github.com/linkerd/linkerd2
RUN mkdir -p web
COPY web/main.go web
Expand Down