Code review comment for lp:~niemeyer/goamz/put-all

LGTM, a few suggestions.

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

https://codereview.appspot.com/7313081/diff/1/s3/multi.go#newcode227
s3/multi.go:227: var listPartsMax = 1000
Is this an absolute limit of all parts per client connection?

https://codereview.appspot.com/7313081/diff/1/s3/multi.go#newcode282
s3/multi.go:282: if totalSize == 0 || err != nil {
On 2013/02/12 07:32:38, jameinel wrote:
> What will this return if totalSize is 0? the return of ListParts? But
doesn't
> that violate the 'you should return nil, otherwise the caller gets an
interface
> object that contains nil but isn't nil'?

> So I would probably split this:
> if err != nil {
> return nil, err
> } else if totalSize == 0 {
> return nil, nil
> }

> Though since you don't have a result object, wouldn't you want a
non-nil err?
> Otherwise you'll get a segfault accessing the result later.

ISTM if totalSize == 0 it will return nil, nil, since there won't be an
error. But if it's not obvious, +1 on the suggestion.

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

https://codereview.appspot.com/7313081/diff/1/s3/multi_test.go#newcode53
s3/multi_test.go:53: c.Assert(multi.UploadId, Matches,
"JNbR_[A-Za-z0-9.]+QQ--")
Maybe move this in assertId()? This seems to be used multiple times, so
it'll be better to have it at one place, I think.

https://codereview.appspot.com/7313081/