Skip to content
This repository has been archived by the owner on Jan 16, 2021. It is now read-only.

Fix govet error in httputil/buster_test.go (copy lock value) #416

Closed
dmitris opened this issue Jun 16, 2016 · 3 comments
Closed

Fix govet error in httputil/buster_test.go (copy lock value) #416

dmitris opened this issue Jun 16, 2016 · 3 comments

Comments

@dmitris
Copy link
Contributor

dmitris commented Jun 16, 2016

Currently running go tool vet . on gddo code produces the following error in httputil/buster_test.go:

$ go tool vet .
buster_test.go:23: assignment copies lock value to cbs: httputil.CacheBusters contains sync.Mutex

would it be OK to change the offending line in https://opensource.git.corp.yahoo.com/golang/gddo/blob/master/httputil/buster_test.go#L23

cbs = CacheBusters{Handler: ss.FileHandler("buster_test.go")}

to cbs.Handler = ss.FileHandler("buster_test.go") ? The tests run fine.

Another possibility would be to create a new variable ex. cbs2 instead of reusing cbs, with the diff of two lines:

-       cbs = CacheBusters{Handler: ss.FileHandler("buster_test.go")}
+       cbs2 := CacheBusters{Handler: ss.FileHandler("buster_test.go")}

-       token = cbs.Get("/xxx")
+       token = cbs2.Get("/xxx")

go tool vet issues cause a problem for CI systems that fail the builds on vet errors (when building local versions of "gddo" using this codebase).

I'd be happy to submit a change, let me know if it wold be ok and if you have any preferences in terms of the options above. Thanks.

@dmitris
Copy link
Contributor Author

dmitris commented Jun 20, 2016

@dmitshur
Copy link
Contributor

dmitshur commented Jun 20, 2016

Fixing a go vet issue (even though this one looks harmless/false positive, see golang/go#13675) is definitely a good idea and appreciated.

I've offered an alternative way to resolve it in a comment at https://go-review.googlesource.com/#/c/24260/1. (I made a typo there, it should be token := cbs.Get("/xxx") in the second block.)

@dmitshur
Copy link
Contributor

This has been fixed in aaf2465, so I'll close the issue.

Thanks for contributing!

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

No branches or pull requests

2 participants