Hi Jeeva, Thanks for sharing go-resty here. I’d like to mention that my input about struct embedding may not be valid. It was pointed out to me previously when I made a similar suggestion that in library design embedding exports all of the embedded type’s methods, so you may have those named fields because of that.
Thanks, Matt On Thursday, January 25, 2018 at 8:53:33 PM UTC-6, Jeevanandam M. wrote: > > Thank you Matt for your inputs and suggestions. I will look into it. > > Cheers, > Jeeva > > > On Thursday, January 25, 2018 at 3:36:44 PM UTC-8, matthe...@gmail.com > wrote: >> >> Hi Jeeva, here’s a code review. >> >> In client.go *Client R() the creation of the *Request unnecessarily sets >> zero values for fields. They could just be omitted instead. Same in >> default.go at func createClient. >> >> The Client type could have *log.Logger and http.Header embedded in the >> struct instead of named. This would shorten calls elsewhere (like c.Printf >> instead of c.Log.Printf). >> >> In middleware.go func parseRequestBody using else instead of goto may be >> clearer. >> >> In redirect.go I’m not understanding the RedirectPolicy interface + >> RedirectPolicyFunc type. Why not just have “type RedirectPolicy func(req >> *http.Request, via []*http.request) error”? >> >> Request could have multiple exported fields embedded. Perhaps Request in >> Response too. >> >> There are a lot of tests, nice. >> >> I’m not sure why there’s a utility valueOf func instead of just calling >> reflect.ValueOf directly. >> >> For documentation I feel that Client has too many functions, methods, and >> fields, but I’m not sure what an alternative would look like. Perhaps these >> types could rely on setting public fields instead of having setters in some >> cases? And maybe Client could be broken into subtypes embedded into the >> Client struct. >> >> Some godoc identifier documentation is missing the period. The README.md >> has many examples which may already be covered by godoc that are making it >> longer than usual. >> >> Matt >> >> On Thursday, January 25, 2018 at 2:27:46 PM UTC-6, Jeevanandam M. wrote: >>> >>> Stable Version : gopkg.in/resty.v1 >>> Edge Version : github.com/go-resty/resty >>> godoc : https://godoc.org/github.com/go-resty/resty >>> >>> >>> Changelog: >>> >>> Features: >>> * Added Request URL Path Params #103 @jeevatkm >>> >>> Enhancements: >>> * Auto detects file content type in mutlipart/form-data mode #109, PR >>> #111 @gh0st42 >>> * Limit body size for debug log PR #99 @sudo-suhas >>> * Log prefix enhancement #113 @jeevatkm >>> * More friendly with mocking test libraries >>> >>> I appreciate your support & feedback! >>> >>> Cheers, >>> Jeeva >>> >> -- 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.
