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 staticcheck to CI, and make the code staticcheck-clean. #22

Closed
danderson opened this issue Feb 11, 2020 · 1 comment
Closed

Add staticcheck to CI, and make the code staticcheck-clean. #22

danderson opened this issue Feb 11, 2020 · 1 comment
Assignees
Labels

Comments

@danderson
Copy link
Member

danderson commented Feb 11, 2020

staticcheck is currently very mad at our code, so we can't add it to CI.

┆Issue is synchronized with this Asana task by Unito

@danderson
Copy link
Member Author

Ground down staticcheck to 3 clusters of errors:

logtail/logtail.go:235:5: var clientSentinelPrefix is unused (U1000)
logtail/logtail.go:238:2: const noSentinel is unused (U1000)
logtail/logtail.go:239:2: const stopSentinel is unused (U1000)
logtail/logtail.go:246:6: func newSentinel is unused (U1000)
logtail/logtail.go:260:6: func readSentinel is unused (U1000)
logtail/logtail.go:346:5: var errHasLogtail is unused (U1000)

Pushed to #23 for a sanity check with code's author.

ipn/ipnserver/server.go:88:2: this value of ctx is never used (SA4006)

I'm tired but I think this is a roundabout way of ensuring that ctx, cancel are always initialized in the following loop, with contortions imposed by Go's shadowing rules. Correct solution is to refactor that whole bit, which will happen because I want to support multiple frontends to a single backend. So maybe we just lint:ignore this until it gets removed.

safesocket/basic_test.go:20:2: the goroutine calls T.Fatal, which must be called in the same goroutine as the test (SA2002)
safesocket/basic_test.go:20:2: the goroutine calls T.Fatalf, which must be called in the same goroutine as the test (SA2002)

Legit logic error in the test. Needs to be refactored such that the failure bubbles up in the correct way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants