Changes LGTM, I've got a couple of suggested tweaks (including
re-joining the revision history of client/client.go), but will make
those myself and propose/land.
https://codereview.appspot.com/6962052/diff/9001/client/local_test.go#newcode47
client/local_test.go:47: + s.Server.URL, //public
Rather than relying on the order here and trusting the comments remain
in sync with the code, I'd prefer using the named field idiom, which
self-documents and is checked by the compiler:
AdminURL: s.server.URL,
InternalURL: s.server.URL,
PublicURL: s.server.URL,
Changes LGTM, I've got a couple of suggested tweaks (including
re-joining the revision history of client/client.go), but will make
those myself and propose/land.
https:/ /codereview. appspot. com/6962052/ diff/9001/ client/ local_test. go local_test. go (right):
File client/
https:/ /codereview. appspot. com/6962052/ diff/9001/ client/ local_test. go#newcode47 local_test. go:47: + s.Server.URL, //public
client/
Rather than relying on the order here and trusting the comments remain
in sync with the code, I'd prefer using the named field idiom, which
self-documents and is checked by the compiler:
AdminURL: s.server.URL,
InternalURL: s.server.URL,
PublicURL: s.server.URL,
https:/ /codereview. appspot. com/6962052/ diff/9001/ identity/ legacy_ test.go legacy_ test.go (right):
File identity/
https:/ /codereview. appspot. com/6962052/ diff/9001/ identity/ legacy_ test.go# newcode27 legacy_ test.go: 27: + map[string] string{ "compute" : management/ url/compute", "object-store": management/ url/object- store"}) test.invalid/...
identity/
"http://
"http://
When making up urls for testing, I like using http://
which you can trust will raise an error on dns lookup if the test
accidentally tries to resolve it.
https:/ /codereview. appspot. com/6962052/ diff/9001/ swift/live_ test.go
File swift/live_test.go (right):
https:/ /codereview. appspot. com/6962052/ diff/9001/ swift/live_ test.go# newcode208 test.go: 208: s.swift. DeleteObject( s.containerName , files[i])
swift/live_
Reasonable use of Check above to ensure the DeleteObject calls should
always happen, test might be clearer with comment to that effect though.
https:/ /codereview. appspot. com/6962052/ diff/9001/ swift/swift. go
File swift/swift.go (left):
https:/ /codereview. appspot. com/6962052/ diff/9001/ swift/swift. go#oldcode33
swift/swift.go:33: // with some refactoring, but for now just make
everything public.
This comment wants updating/removing now I'd say.
https:/ /codereview. appspot. com/6962052/