ringpop-go icon indicating copy to clipboard operation
ringpop-go copied to clipboard

`go tool vet` is not running on all examples

Open thanodnl opened this issue 8 years ago • 3 comments

When running make lint the vetting tool is not ran on all our examples by default. This let a vetting error slip through the cracks in the ping-thrift-gen example.

For some reason go tool vet only runs on this example after installing the example:

$ go install github.com/uber/ringpop-go/examples/ping-thrift-gen/

When you run make lint after an installation you will see the following error

$ make lint
ERROR: ./examples/ping-thrift-gen/main.go:95: github.com/uber/ringpop-go/examples/ping-thrift-gen/gen-go/ping.Pong composite literal uses unkeyed fields
make: *** [lint] Error 1

This will prevent the precommit hook that is setup to finish which makes it harder to commit. The easiest way to remove this error is by removing the ping-thrift-gen from pkg via

$ rm -rf $GOPATH/pkg/darwin_amd64/github.com/uber/ringpop-go/examples/ping-thrift-gen # OSX specific

We should find the root cause for the vetter to not be running correctly and fix the vetting issue that it finds when it does run.

thanodnl avatar Jul 28 '16 14:07 thanodnl

The error in question is a valid error, it's complaining about: https://github.com/uber/ringpop-go/blob/master/examples/ping-thrift-gen/main.go#L95

    return &gen.Pong{"Hello, world!", identity, &pHeader}, nil

The vet tool raises an error when you don't specify keys for literals for types that are defined outside of your package. You can fix this by specifying keys (e.g., &gen.Pong{Message: "Hello, world!"}).

The reason this is an error is to make it easier for package owners to add fields to structs without breaking all code that depends on this package. If some package uses B.Pong{"Hello world"}, now that call will fail when the owner of B adds a new field to Pong, since unkeyed literals must specify all fields.

The vet tool uses the package cache to determine the types. E.g., if the Pong type was actually a slice, then you wouldn't need keys.

prashantv avatar Jul 28 '16 14:07 prashantv

Thanks for the explanation why it needs the files in pkg. Now we need to find a way to get these files available on travis-ci so the make lint test starts failing. Because now we ran into this 'by accident' just wondering if there are more (important) files failing on vet as well.

thanodnl avatar Jul 28 '16 16:07 thanodnl

To get packages into the pkg directory, you need to do go install, or go test -i or go build -i.

In TChannel, we run the tests first, which will do go test -i to install all the dependencies and store them in pkg, then we run the lint rules so vet has the package files available.

I still need to write that common Makefile or simple build system for our open source repositories so we don't have to duplicate Makefiles everywhere, just haven't gotten the chance yet

prashantv avatar Jul 28 '16 16:07 prashantv