Improve syntax for specifying empty string values #7615

Merged
merged 1 commit into from Jul 11, 2017

Conversation

Projects
None yet
3 participants
Owner

wallyworld commented Jul 11, 2017

Description of change

The syntax for setting a model config to empty string is clumsy, requiring the "" to be escaped, ie ""
This makes it so that you can now use foo="" or even foo=

QA steps

$ juju model-config http-proxy=something
$ juju model-config http-proxy=""

Documentation changes

We can tweak the model-config doc

Bug reference

https://bugs.launchpad.net/juju/+bug/1703480

axw approved these changes Jul 11, 2017

cmd/juju/common/flags.go
- if err := yaml.Unmarshal([]byte(fields[1]), &value); err != nil {
- return errors.Trace(err)
+ if fields[1] == "" {
+ value = fields[1]
@axw

axw Jul 11, 2017

Member

seeing as you've already assert that it's "", I think it would be clearer if you made this value = ""

cmd/juju/common/flags_test.go
+ assertConfigFlag(c, f, []string{"a.yaml", "b.yaml"}, map[string]interface{}{"k1": ""})
+ c.Assert(f.Set("k1=v1"), jc.ErrorIsNil)
+ assertConfigFlag(c, f, []string{"a.yaml", "b.yaml"}, map[string]interface{}{"k1": "v1"})
+ c.Assert(f.Set(`k1=`), jc.ErrorIsNil)
@axw

axw Jul 11, 2017

Member

this is exactly the same as the one on line 33

@wallyworld

wallyworld Jul 11, 2017

Owner

ah, yeah, should be k1=""

Owner

wallyworld commented Jul 11, 2017

$$merge$$

Contributor

jujubot commented Jul 11, 2017

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

@jujubot jujubot merged commit 262638a into juju:2.2 Jul 11, 2017

1 check passed

github-check-merge-juju Ran tests against PR. Use !!.*!! to request another build. IE, !!build!!, !!retry!!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment