Code review comment for lp:~themue/juju-core/032-config-default-empty-string

Please take a look.

https://codereview.appspot.com/10682043/diff/1/charm/config.go
File charm/config.go (left):

https://codereview.appspot.com/10682043/diff/1/charm/config.go#oldcode117
charm/config.go:117: }
On 2013/06/28 09:32:11, fwereade wrote:
> I would personally find something like this a bit easier to follow:

> def := option.Default
> if def == "" && option.Type == "string" {
> // Skip normal validation for compatibility with python.
> } else if option.Default, err := option.validate(name, def); err !=
nil {
> option.error(&err, name, def)
> return nil, fmt.Errorf("invalid config default: %v", err)
> }

> but YMMV, I guess, and there's no point bikeshedding this too hard,
because
> we'll have to deal with ""-everywhere before too long.

Done.

https://codereview.appspot.com/10682043/diff/1/charm/config.go
File charm/config.go (right):

https://codereview.appspot.com/10682043/diff/1/charm/config.go#newcode114
charm/config.go:114: // Empty default string is an exceptional case due
to compatability reasons.
On 2013/06/27 15:01:20, dimitern wrote:
> s/compatability/compatibility/ reasons with pyjuju.

Changed with Williams proposal.

https://codereview.appspot.com/10682043/diff/1/charm/config_test.go
File charm/config_test.go (right):

https://codereview.appspot.com/10682043/diff/1/charm/config_test.go#newcode103
charm/config_test.go:103: "subtitle": "",
On 2013/06/28 09:32:11, fwereade wrote:
> Just skip this. You haven't changed the behaviour of FilterSettings,
and you're
> just duplicating a case that's already tested by "outlook".

Done.

https://codereview.appspot.com/10682043/