Code review comment for lp:~dimitern/goamz/subnets-api-calls

Revision history for this message
Dimiter Naydenov (dimitern) wrote :

Please take a look.

https://codereview.appspot.com/54690048/diff/40001/ec2/ec2test/server.go
File ec2/ec2test/server.go (right):

https://codereview.appspot.com/54690048/diff/40001/ec2/ec2test/server.go#newcode1105
ec2/ec2test/server.go:1105: availIPs :=
int(math.Exp2(float64(maskBits-maskOnes))) - 5
On 2014/02/05 14:46:53, rog wrote:
> availIPs := 1<<uint(maskBits - maskOnes) - 5
> ?

> i suppose we might consider making sure that there
> are enough bits in the mask too, otherwise
> we can get a negative AvailableIPCount.

Done.

https://codereview.appspot.com/54690048/diff/40001/ec2/subnets.go
File ec2/subnets.go (right):

https://codereview.appspot.com/54690048/diff/40001/ec2/subnets.go#newcode42
ec2/subnets.go:42: // When you create each subnet, you provide the VPC
ID and the CIDR
On 2014/02/05 14:46:53, rog wrote:
> // The vpcId and cidrBlock parameters specify the
> // VPC id and CIDR block respectively - these cannot
> // be changed after creation. The subnet's CIDR block can be the same
as
> // the VPC's CIDR block (assuming you want only a single subnet in the
> // VPC), or a subset of the VPC's CIDR block. If more than one
> // subnet is created in a VPC, their CIDR blocks must not overlap.
The
> // smallest subnet (and VPC) that can be created uses a /28 netmask
(16 IP
> // addresses), and the largest uses a /16 netmask (65,536 IP
> // addresses).

> ?

Done.

https://codereview.appspot.com/54690048/diff/40001/ec2/subnets.go#newcode52
ec2/subnets.go:52: // availZone can be empty, in which case Amazon EC2
selects one for
On 2014/02/05 14:46:53, rog wrote:
> s/can/may/

Done.

https://codereview.appspot.com/54690048/diff/40001/ec2/subnets.go#newcode71
ec2/subnets.go:71: // DeleteSubnet deletes the specified subnet. You
must terminate all
On 2014/02/05 14:46:53, rog wrote:
> // All running instances in the subnet must have been terminated.

> ?

Done.

https://codereview.appspot.com/54690048/

« Back to merge proposal