Code review comment for lp:~jameinel/goose/transfer-content-length-1124561

Revision history for this message
John A Meinel (jameinel) wrote :

Please take a look.

https://codereview.appspot.com/10465043/diff/1/http/client_test.go
File http/client_test.go (right):

https://codereview.appspot.com/10465043/diff/1/http/client_test.go#newcode59
http/client_test.go:59: headers := http.Header{}
On 2013/06/26 23:48:37, dfc wrote:
> var headers http.Header

Care to explain why one would be preferred to the other?

https://codereview.appspot.com/10465043/diff/1/http/client_test.go#newcode60
http/client_test.go:60: emptyBody := "<no-response-yet>"
On 2013/06/26 14:59:43, gz wrote:
> Why not just the empty string here?

I was distinguishing between "the body read from remote was empty" and
"I didn't get to the point of reading the content".

I can probably switch back, though.

https://codereview.appspot.com/10465043/diff/1/http/client_test.go#newcode110
http/client_test.go:110: c.Assert(body, NotNil)
On 2013/06/26 23:48:37, dfc wrote:
> this is weird, i cannot see how body will ever not be nil

body is a pointer to a string, and when we get the response from the
server, we point that to the contents of the request.

Why would it be nil?

https://codereview.appspot.com/10465043/

« Back to merge proposal