Code review comment for lp:~nick-craig-wood/goamz/list-buckets

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

Looking good, thanks for this. Just a few trivial details to sort out.

https://codereview.appspot.com/7305051/diff/1/s3/s3.go
File s3/s3.go (right):

https://codereview.appspot.com/7305051/diff/1/s3/s3.go#newcode43
s3/s3.go:43: // The following fields are only set on Buckets returned
from the ListBuckets call
s/Buckets/buckets/

https://codereview.appspot.com/7305051/diff/1/s3/s3.go#newcode45
s3/s3.go:45: CreationDate string // Date the bucket was created, e.g.,
2009-02-03T16:45:09.000Z
Seems like this should be a native time.Time.

https://codereview.appspot.com/7305051/diff/1/s3/s3.go#newcode74
s3/s3.go:74: type BucketInfo struct {
s/BucketInfo/bucketInfo/

This is used only internally, so doesn't have to be exposed in the
public API.

https://codereview.appspot.com/7305051/diff/1/s3/s3.go#newcode80
s3/s3.go:80: type Buckets struct {
s/Buckets/bucketInfoResp/, for the same reason.

The doc also needs fixing.

https://codereview.appspot.com/7305051/diff/1/s3/s3.go#newcode444
s3/s3.go:444: if req.baseurl == "" {
That logic is changed in trunk, both due to retries and to fix a bug.
Trunk needs to be merged back so we can see what the actual change
needed is here.

https://codereview.appspot.com/7305051/

« Back to merge proposal