Thank you Matt. Cheers, Jeeva
On Saturday, January 27, 2018 at 6:25:17 AM UTC-8, matthe...@gmail.com wrote: > > 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.
