vet complains :'(
yields opened this issue · 8 comments
func main(){
stats.Incr("user.login", stats.Tag{"foo", "baz"})
// stats.Tag composite literal uses unkeyed fields
}
doesn't make sense since there are just two fields, not sure why vet
thinks it's a bid deal.
I feel like go vet
is not very predictable, there are cases where it doesn't complain about the use of this notation, sometime it does.
You can fix it by doing stats.Tag{Name: "foo", Value: "baz"}
... that's more verbose.
Maybe we can introduce a function like this:
func T(name string, value string) Tag {
return Tag{Name: name, Value: value}
}
Then the code could be made shorter:
stats.Incr("user.login", stats.T("foo", "baz"))
and vet won't complain anymore!
How do you feel about going with a map, my thinking is that it's rare to go with a tag like foo
, most resources i've seen use tags like foo:baz
, a map also allows us to add multiple tags easier:
stats.Incr("foo.baz", stats.T{"foo": "baz"})
a bit more verbose than a function though, plus it's rare to add multiple tags per metric.
though this breaks the current api, so not sure.
I feel like it's a little late to break the API, plus maps always cause dynamic memory allocations and are slower to initialize and stats tend to be on pretty hot paths of the program (otherwise we'd measure useless stuff).
I'm gonna think a little more about this.
i've definitely seen dumb vet
complaints before, but this takes the cake, i'm considering just dropping it.
do you use it at all?
I use it but not religiously, when it goes rogue I just ignore it or work around it if it's easy.
In the build container it's simply shown as a warning, it doesn't block the build.
You can disable this via -composites=false
.
$ go doc cmd/vet
Unkeyed composite literals
Flag: -composites
Composite struct literals that do not use the field-keyed syntax.
Closing this issue as I don't think there is any follow up action to take in the library itself?
Thanks for the follow up! It's good to know!