Code review comment for lp:~themue/juju-core/go-ec2-security-group-config

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

LGTM, but please add a test where indicated.

https://codereview.appspot.com/6596051/diff/3001/environs/config/config.go
File environs/config/config.go (right):

https://codereview.appspot.com/6596051/diff/3001/environs/config/config.go#newcode10
environs/config/config.go:10: // per machine of global.
// FirewallMode defines the way in which the environment
// handles opening and closing of firewall ports.

https://codereview.appspot.com/6596051/diff/3001/environs/config/config.go#newcode13
environs/config/config.go:13: const (
// FwDefault is the environment-specific default mode.

https://codereview.appspot.com/6596051/diff/3001/environs/config/config.go#newcode14
environs/config/config.go:14: FwDefault FirewallMode = "default"
<empty line>
// FwGlobal requests the use of a single firewall group for all
machines.
// When ports are opened for one machine, all machines will have the
same
// port opened.

https://codereview.appspot.com/6596051/diff/3001/environs/config/config_test.go
File environs/config/config_test.go (right):

https://codereview.appspot.com/6596051/diff/3001/environs/config/config_test.go#newcode133
environs/config/config_test.go:133: },
We need a test here.

https://codereview.appspot.com/6596051/

« Back to merge proposal