Merge lp://qastaging/~evarlast/charm-tools/warn-description-newlines into lp://qastaging/charm-tools/1.4

Proposed by Jay R. Wren
Status: Rejected
Rejected by: Marco Ceppi
Proposed branch: lp://qastaging/~evarlast/charm-tools/warn-description-newlines
Merge into: lp://qastaging/charm-tools/1.4
Diff against target: 73 lines (+52/-0)
2 files modified
charmtools/charms.py (+5/-0)
tests/test_charm_proof.py (+47/-0)
To merge this branch: bzr merge lp://qastaging/~evarlast/charm-tools/warn-description-newlines
Reviewer Review Type Date Requested Status
Charm Toolers Pending
Review via email: mp+230899@code.qastaging.launchpad.net

Description of the change

  Warn on newlines in config description

  If there are newlines in a config options description,
  juju get <charmname> displays the newlines making for very ugly
  formatting.

  This patch causes charm-proof to warn on newlines in options description.

See also: https://bugs.launchpad.net/juju-core/+bug/1292116

To post a comment you must log in.
Revision history for this message
Charles Butler (lazypower) wrote :

+1 LGTM

Revision history for this message
Marco Ceppi (marcoceppi) wrote :

NLGTM

Revision history for this message
Tim Van Steenburgh (tvansteenburgh) wrote :

I appreciate this patch very much, but I don't think this is the correct way to solve this problem.

I think imposing arbitrary restrictions on which yaml contructs can be used will lead to a surprising user experience. This problem should be addressed by fixing the way the data is displayed by `juju get`, not by imposing restrictions on the data itself.

Revision history for this message
Charles Butler (lazypower) wrote :

> +1 LGTM

And I've since been wayyy corrected about why this is not good. DNLGTM

Unmerged revisions

338. By Jay R. Wren

Warn on newlines in config description

If there are newlines in a config options description,
juju get <charmname> displays the newlines making for very ugly
formatting.

This patch causes charm-proof to warn on newlines in options description.

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

to all changes: