Sorry, NOT LGTM without explanation. The API has lost/changed
functionality, I think, because we can no longer set string values to
defaults; and we've now got two ways to set defaults for non-string
types. How is all this going to work in the imminent CLI-API world?
https://codereview.appspot.com/12343045/diff/1/charm/config.go#newcode48
charm/config.go:48: } else if option.Type != "string" && value == "" {
Why are we keeping this behaviour? Having `value=` mean different things
depending on type seems like a bad idea to me. Can't we just delete this
whole special-case block?
https://codereview.appspot.com/12343045/diff/1/charm/config_test.go#newcode337
charm/config_test.go:337: info: "empty strings are nil values for
non-string types",
Yeah -- why do we now have two ways of specifying set-to-default? I
don't think this is a good idea *unless* we have to support `value=` for
python compatibility.
Sorry, NOT LGTM without explanation. The API has lost/changed
functionality, I think, because we can no longer set string values to
defaults; and we've now got two ways to set defaults for non-string
types. How is all this going to work in the imminent CLI-API world?
https:/ /codereview. appspot. com/12343045/ diff/1/ charm/config. go
File charm/config.go (right):
https:/ /codereview. appspot. com/12343045/ diff/1/ charm/config. go#newcode39
charm/config.go:39: // are always considered valid, and empty string
values are converted to nil.
This isn't true any more.
https:/ /codereview. appspot. com/12343045/ diff/1/ charm/config. go#newcode48
charm/config.go:48: } else if option.Type != "string" && value == "" {
Why are we keeping this behaviour? Having `value=` mean different things
depending on type seems like a bad idea to me. Can't we just delete this
whole special-case block?
https:/ /codereview. appspot. com/12343045/ diff/1/ charm/config. go#newcode67
charm/config.go:67: if option.Type != "string" && str == "" {
Similarly.
https:/ /codereview. appspot. com/12343045/ diff/1/ charm/config_ test.go test.go (right):
File charm/config_
https:/ /codereview. appspot. com/12343045/ diff/1/ charm/config_ test.go# newcode337 test.go: 337: info: "empty strings are nil values for
charm/config_
non-string types",
Yeah -- why do we now have two ways of specifying set-to-default? I
don't think this is a good idea *unless* we have to support `value=` for
python compatibility.
https:/ /codereview. appspot. com/12343045/ diff/1/ state/statecmd/ config_ test.go config_ test.go (right):
File state/statecmd/
https:/ /codereview. appspot. com/12343045/ diff/1/ state/statecmd/ config_ test.go# newcode90 config_ test.go: 90: "value": "",
state/statecmd/
So how do we set defaults over the API?
https:/ /codereview. appspot. com/12343045/