[go-nuts] Re: [ANN] go-resty v1.1 released - Simple HTTP and REST client library
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.
[go-nuts] Re: [ANN] go-resty v1.1 released - Simple HTTP and REST client library
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.
[go-nuts] Re: [ANN] go-resty v1.1 released - Simple HTTP and REST client library
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.
[go-nuts] Re: [ANN] go-resty v1.1 released - Simple HTTP and REST client library
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.
