Thanks for the CR Matt, that's very kind of you. About the factory, I thought that some of the options would be sometimes easier to define in construction time, and copy the object doesn't add complexity to my opinion.
About two sets of optoins, except of the "WithName" option, there is no "timingOptions", but it is interesting that you brought it up: I initially intended to do something more general than only http client, for example, a timing client that could behave similar for http client and for sql connection. I meet two problems: 1. It would be hard to define an interface that will be common: one would interact with http request/response and one with sql query and arguments. 2. The sql package does not provide any sort of "RoundTripper" interface, that could be used to wrap the sql calls, I found an open issue in github <https://github.com/golang/go/issues/14468> about something similar to my problem. On Monday, February 26, 2018 at 5:56:28 PM UTC+2, matthe...@gmail.com wrote: > > Hi Eyal, here’s a code review. > > Perhaps there’s a simpler name? Something like “timing.Timer” would look > better than “clienttiming.Timer”. > > return fmt.Sprintf("%s %s", req.Method, req.URL.Path) > > could just be > > return req.Method + “ “ + req.URL.Path > > which removes the package fmt dependency. > > The godoc summary could be improved with a bullet list or by rewriting “It > provides:\n\n…”. > > Some of the godoc identifier descriptions are missing the period. > > I’m not sure about the Timer factory for http.Client and > http.RoundTripper. I can see that two sets of options need to be provided, > but the package API could be reduced by doing something like: > > func NewClient(ctx context.Context, timingOpts []Option, httpOpts []Option > ) *http.Client > > There is a tradeoff in readability though. Maybe using the options pattern > is adding unnecessary complexity here? > > Thanks for the MIT licensing. > > Matt > > On Monday, February 26, 2018 at 4:16:04 AM UTC-6, Eyal Posener wrote: >> >> Hi, >> Recently mitchellh wrote a really awesome library >> <https://github.com/mitchellh/go-server-timing> that provide HTTP >> middleware for server-timing headers. >> I saw that and thought it would be really nice to automate those headers >> for HTTP calls between servers. >> >> So I created this library: https://github.com/posener/client-timing. >> >> - An HTTP Client or RoundTripper, fully compatible with Go's standard >> library. >> - Automatically time HTTP requests sent from an HTTP handler. >> - Collects all timing headers from upstream servers. So if you called >> server A, A called B and B called C, you'll get all the information in >> the >> response, assuming all the servers used the middleware and the timing >> client. >> - Customize timing headers according to the request, response and >> error of the HTTP round trip. >> >> Would love to hear your feedback about it. >> Cheers, >> Eyal >> > -- 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.
