Thanks much, will review this, and incorporate changes... On Wednesday, May 17, 2017 at 12:56:59 AM UTC-7, Lutz Horn wrote: > > Hi, > > the more I work with Go the more I like it. Reading code written by > other people, even by beginners, is straight forward. > > > https://github.com/kbfastcat/nrmetrics > > go_vet and the misspell check found some minor issues: > > https://goreportcard.com/report/github.com/kbfastcat/nrmetrics > > My observations: > > -- > > In the lines > > https://github.com/kbfastcat/nrmetrics/blob/master/nrpost/nrpost.go#L19 > > https://github.com/kbfastcat/nrmetrics/blob/master/nrpost/nrpost.go#L30 > > > https://github.com/kbfastcat/nrmetrics/blob/master/redis-agent/config.go#L27 > > > https://github.com/kbfastcat/nrmetrics/blob/master/redis-agent/config.go#L33 > > I would > > return nil, err > > instead of an unused first item. > > -- > > Why don't you export metric on > > > https://github.com/kbfastcat/nrmetrics/blob/master/redis-agent/getstats.go#L38 > > > when you do export the other types? > > -- > > Could you write tests for parseInt64 and parseFloat64 that actually have > multiple lines as the info argument? The slice you get at > > > https://github.com/kbfastcat/nrmetrics/blob/master/redis-agent/getstats.go#L127 > > > only contains one element in your tests. > > -- > > The names parseInt64 and parseFloat64 are a little misleading. I would > expect them to only accept on string argument that contains an int64 or > a float64 plus some to be ignored characters. But your functions look > for the marker given as the s argument and then get the number right of > the colon. > > Maybe names that better tell the reader what the functions are doing > could be chosen. > > -- > > The help strings of the two flags at > > > https://github.com/kbfastcat/nrmetrics/blob/master/redis-agent/redis-agent.go#L21 > > > just duplicate the flag names in a unhelpful way. You should describe > the meaning of the flags. > > -- > > I am sure others here can give you more helpful advice. But this what I, > a beginner like you, find from reading your code. > > Lutz > > >
-- You received this message because you are subscribed to the Google Groups "golang-nuts" group. To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
