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

Deprecated warnings are not properly ignored #751

Closed
bentucker opened this issue May 13, 2020 · 4 comments
Closed

Deprecated warnings are not properly ignored #751

bentucker opened this issue May 13, 2020 · 4 comments

Comments

@bentucker
Copy link

In both master (ce2dae2) and v1.6.0 (6edbbd9), make fails on golangci-lint. It seems the staticcheck ignores are not being respected.

>> checking code style
>> checking license header
>> running golangci-lint
GO111MODULE=on go list -e -compiled -test=true -export=false -deps=true -find=false -tags= -- ./... > /dev/null
GO111MODULE=on /home/magma/go/bin/golangci-lint run  ./...
prometheus/desc.go:23:2: SA1019: package github.com/golang/protobuf/proto is deprecated: Use the "google.golang.org/protobuf/proto" package instead.  (staticcheck)
        "github.com/golang/protobuf/proto"
        ^
prometheus/histogram.go:25:2: SA1019: package github.com/golang/protobuf/proto is deprecated: Use the "google.golang.org/protobuf/proto" package instead.  (staticcheck)
        "github.com/golang/protobuf/proto"
        ^
prometheus/metric.go:20:2: SA1019: package github.com/golang/protobuf/proto is deprecated: Use the "google.golang.org/protobuf/proto" package instead.  (staticcheck)
        "github.com/golang/protobuf/proto"
        ^
prometheus/promhttp/delegator.go:88:27: SA1019: http.CloseNotifier is deprecated: the CloseNotifier interface predates Go's context package. New code should use Request.Context instead.  (staticcheck)
        return d.ResponseWriter.(http.CloseNotifier).CloseNotify()
                                 ^
prometheus/promhttp/delegator.go:353:17: SA1019: http.CloseNotifier is deprecated: the CloseNotifier interface predates Go's context package. New code should use Request.Context instead.  (staticcheck)
        if _, ok := w.(http.CloseNotifier); ok {
                       ^
prometheus/promhttp/instrument_server_test.go:299:5: SA1019: http.CloseNotifier is deprecated: the CloseNotifier interface predates Go's context package. New code should use Request.Context instead.  (staticcheck)
        d.(http.CloseNotifier).CloseNotify()
           ^
make: *** [Makefile.common:181: common-lint] Error 1```
@simonpasquier
Copy link
Member

I suspect that you've got a recent version of golangci-lint which doesn't recognize the //lint:ignore comments. I've already reported the issue upstream (golangci/golangci-lint#741) but nothing has changed since then. As commented here, maybe we should run the original staticcheck and disable it in golangci-lint.

@beorn7
Copy link
Member

beorn7 commented May 14, 2020

The protobuf warnings are actually legitimate. We either need to migrate to the new package (which is not a drop-in replacement), or we have to add more "ignore" comments.

@beorn7
Copy link
Member

beorn7 commented May 14, 2020

DId the latter in #754 as we cannot really migrate without a breaking change for the users of this repo.

@beorn7
Copy link
Member

beorn7 commented May 18, 2020

More lint:ignores in place. Our CI is fine (we are using an older glangci-lint version, I guess?). staticcheck locally is fine. The remaining issue is for golangci-lint to fix. I'll close this issue now.

@beorn7 beorn7 closed this as completed May 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants