LGTM except for the query about the default security group test.
nice to see all that code factored out.
https://codereview.appspot.com/7232065/diff/1/ec2/ec2_test.go File ec2/ec2_test.go (right):
https://codereview.appspot.com/7232065/diff/1/ec2/ec2_test.go#newcode24 ec2/ec2_test.go:24: testServer.Start() that makes much more sense, given we're using the global below.
https://codereview.appspot.com/7232065/diff/1/ec2/ec2t_test.go File ec2/ec2t_test.go (right):
https://codereview.appspot.com/7232065/diff/1/ec2/ec2t_test.go#newcode447 ec2/ec2t_test.go:447: //resp, err := s.ec2.SecurityGroups([]ec2.SecurityGroup{{Name: "default"}}, nil) why commented out? this was testing specific ec2test functionality.
have you tried these tests live? i seem to remember that i needed to add the "default" group because it was being returned for some of the filtering requests.
https://codereview.appspot.com/7232065/diff/1/ec2/ec2test/server.go File ec2/ec2test/server.go (right):
https://codereview.appspot.com/7232065/diff/1/ec2/ec2test/server.go#newcode420 ec2/ec2test/server.go:420: // ImageId: accept anything, we can verify later align? (and below)
https://codereview.appspot.com/7232065/diff/1/ec2/sign.go File ec2/sign.go (right):
https://codereview.appspot.com/7232065/diff/1/ec2/sign.go#newcode23 ec2/sign.go:23: // be in natural order of the keys. This is distinct from the s/in natural/in the natural/ ?
https://codereview.appspot.com/7232065/diff/1/exp/mturk/suite_test.go File exp/mturk/suite_test.go (left):
https://codereview.appspot.com/7232065/diff/1/exp/mturk/suite_test.go#oldcode1 exp/mturk/suite_test.go:1: package mturk_test great to see all this duplication go away.
https://codereview.appspot.com/7232065/diff/1/s3/s3.go File s3/s3.go (right):
https://codereview.appspot.com/7232065/diff/1/s3/s3.go#newcode231 s3/s3.go:231: // into different groupings of keys, similar to how folders would work. s/would// ?
https://codereview.appspot.com/7232065/
« Back to merge proposal
LGTM except for the query about the default security group test.
nice to see all that code factored out.
https:/ /codereview. appspot. com/7232065/ diff/1/ ec2/ec2_ test.go
File ec2/ec2_test.go (right):
https:/ /codereview. appspot. com/7232065/ diff/1/ ec2/ec2_ test.go# newcode24
ec2/ec2_test.go:24: testServer.Start()
that makes much more sense, given we're using the global below.
https:/ /codereview. appspot. com/7232065/ diff/1/ ec2/ec2t_ test.go
File ec2/ec2t_test.go (right):
https:/ /codereview. appspot. com/7232065/ diff/1/ ec2/ec2t_ test.go# newcode447 test.go: 447: //resp, err := roups([ ]ec2.SecurityGr oup{{Name: "default"}}, nil)
ec2/ec2t_
s.ec2.SecurityG
why commented out? this was testing specific ec2test functionality.
have you tried these tests live? i seem to remember that
i needed to add the "default" group because it was
being returned for some of the filtering requests.
https:/ /codereview. appspot. com/7232065/ diff/1/ ec2/ec2test/ server. go server. go (right):
File ec2/ec2test/
https:/ /codereview. appspot. com/7232065/ diff/1/ ec2/ec2test/ server. go#newcode420 server. go:420: // ImageId: accept anything, we can verify
ec2/ec2test/
later
align?
(and below)
https:/ /codereview. appspot. com/7232065/ diff/1/ ec2/sign. go
File ec2/sign.go (right):
https:/ /codereview. appspot. com/7232065/ diff/1/ ec2/sign. go#newcode23
ec2/sign.go:23: // be in natural order of the keys. This is distinct
from the
s/in natural/in the natural/
?
https:/ /codereview. appspot. com/7232065/ diff/1/ exp/mturk/ suite_test. go suite_test. go (left):
File exp/mturk/
https:/ /codereview. appspot. com/7232065/ diff/1/ exp/mturk/ suite_test. go#oldcode1 suite_test. go:1: package mturk_test
exp/mturk/
great to see all this duplication go away.
https:/ /codereview. appspot. com/7232065/ diff/1/ s3/s3.go
File s3/s3.go (right):
https:/ /codereview. appspot. com/7232065/ diff/1/ s3/s3.go# newcode231
s3/s3.go:231: // into different groupings of keys, similar to how
folders would work.
s/would// ?
https:/ /codereview. appspot. com/7232065/