A direct test for the glob change, and a helper function to create the storage are needed.
https://codereview.appspot.com/13736043/diff/4001/environs/httpstorage/backend.go File environs/httpstorage/backend.go (right):
https://codereview.appspot.com/13736043/diff/4001/environs/httpstorage/backend.go#newcode54 environs/httpstorage/backend.go:54: defer readcloser.Close() Why defer? There is no harm in calling Close right now.
https://codereview.appspot.com/13736043/diff/4001/environs/httpstorage/backend.go#newcode75 environs/httpstorage/backend.go:75: if req.ContentLength <= -1 { isn't " < 0 " more readable and understandable?
https://codereview.appspot.com/13736043/diff/4001/environs/httpstorage/backend_test.go File environs/httpstorage/backend_test.go (right):
https://codereview.appspot.com/13736043/diff/4001/environs/httpstorage/backend_test.go#newcode109 environs/httpstorage/backend_test.go:109: // Get on a directory returns a 500 as it is I argue with this status code.
5xx means it is our fault. 4xx means it is your fault. This is certainly a bad request.
https://codereview.appspot.com/13736043/diff/4001/environs/testing/storage.go File environs/testing/storage.go (right):
https://codereview.appspot.com/13736043/diff/4001/environs/testing/storage.go#newcode33 environs/testing/storage.go:33: listener, err := httpstorage.Serve("localhost:0", underlying) Given this is the second or third time I have seen these lines of code, perhaps a testing helper function would be good.
https://codereview.appspot.com/13736043/
« Back to merge proposal
A direct test for the glob change, and a helper function to create the
storage are needed.
https:/ /codereview. appspot. com/13736043/ diff/4001/ environs/ httpstorage/ backend. go httpstorage/ backend. go (right):
File environs/
https:/ /codereview. appspot. com/13736043/ diff/4001/ environs/ httpstorage/ backend. go#newcode54 httpstorage/ backend. go:54: defer readcloser.Close()
environs/
Why defer? There is no harm in calling Close right now.
https:/ /codereview. appspot. com/13736043/ diff/4001/ environs/ httpstorage/ backend. go#newcode75 httpstorage/ backend. go:75: if req.ContentLength <= -1 {
environs/
isn't " < 0 " more readable and understandable?
https:/ /codereview. appspot. com/13736043/ diff/4001/ environs/ httpstorage/ backend_ test.go httpstorage/ backend_ test.go (right):
File environs/
https:/ /codereview. appspot. com/13736043/ diff/4001/ environs/ httpstorage/ backend_ test.go# newcode109 httpstorage/ backend_ test.go: 109: // Get on a directory returns
environs/
a 500 as it is
I argue with this status code.
5xx means it is our fault. 4xx means it is your fault. This is
certainly a bad request.
https:/ /codereview. appspot. com/13736043/ diff/4001/ environs/ testing/ storage. go testing/ storage. go (right):
File environs/
https:/ /codereview. appspot. com/13736043/ diff/4001/ environs/ testing/ storage. go#newcode33 testing/ storage. go:33: listener, err := Serve(" localhost: 0", underlying)
environs/
httpstorage.
Given this is the second or third time I have seen these lines of code,
perhaps a testing helper function would be good.
https:/ /codereview. appspot. com/13736043/