Merge lp://staging/~jorge/charms/trusty/juju-gui/add-default-values into lp://staging/charms/trusty/juju-gui

Proposed by Jorge Castro
Status: Needs review
Proposed branch: lp://staging/~jorge/charms/trusty/juju-gui/add-default-values
Merge into: lp://staging/charms/trusty/juju-gui
Diff against target: 32 lines (+4/-0)
1 file modified
config.yaml (+4/-0)
To merge this branch: bzr merge lp://staging/~jorge/charms/trusty/juju-gui/add-default-values
Reviewer Review Type Date Requested Status
Brad Crittenden (community) Needs Information
Review via email: mp+234029@code.staging.launchpad.net

Description of the change

Add default values to pass charm proof.

To post a comment you must log in.
Revision history for this message
Brad Crittenden (bac) wrote :

Thanks for the changes Jorge.

Unfortunately this version does not pass charm-proof, which is a fault of the proofing tool. Running it I see:

[juju-gui]bac@trusty64:~/charms/trusty/juju-gui$ charm-proof .
I: config.yaml: option password has no default value
I: config.yaml: option ssl-key-contents has no default value
I: config.yaml: option ssl-cert-contents has no default value
I: config.yaml: option login-help has no default value

Null should be a valid default value for strings. It is treated as None when parsed in Python. The tool is satisfied if you use:

default: ""

But the empty string is not the same as None. In our charm we have tests like:

if login_help is None: # Use the default value

Setting the default to the empty string causes this test to fail and no login help is shown.

If we insist that a 'default: ' key be provide for each configuration value then the tool should be updated to accept a null value as represented by:

default:
default: null
default: ~

See http://yaml.org/type/null.html for more details.

If the tool is already being updated or you have more information please let me know.

review: Needs Information

Unmerged revisions

98. By Jorge Castro

Add default values to pass charm proof.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches