Code review comment for lp:~thumper/juju-core/no-proxy

Revision history for this message
Ian Booth (wallyworld) wrote :

LGTM so long as you are sure there are no backwards compatibility issues
with the config schema change

https://codereview.appspot.com/73290043/diff/1/environs/config/config.go
File environs/config/config.go (right):

https://codereview.appspot.com/73290043/diff/1/environs/config/config.go#newcode745
environs/config/config.go:745:
Config schema changes always scare me, especially since we've had back
backwards compatibility issues in the past eg for the simplestreams
change - are there tests required specifically for this change?

https://codereview.appspot.com/73290043/diff/1/juju/osenv/proxy.go
File juju/osenv/proxy.go (right):

https://codereview.appspot.com/73290043/diff/1/juju/osenv/proxy.go#newcode21
juju/osenv/proxy.go:21: // Detect Proxies.
Perhaps update the comment to mention NoProxy as it's quite important

https://codereview.appspot.com/73290043/

« Back to merge proposal