Merge lp:~mariosplivalo/juju-deployer/juju-deployer into lp:juju-deployer

[Mario Splivalo]

--local-mods was enabled by default and there was no way to disable it. Based on the name of the option, it should be disabled by default, and enabled only when --local-mods option is present on the commandline.

[lp:~mariosplivalo/juju-deployer/juju-deployer]

117. By Mario Splivalo on 2014-05-29

The logic was fine, just the help text was wrong :/

[Jorge Niedbalski (niedbalski)]

The original option is semantically confusing, if you pass --local-mods it should means: enable local modifications and it should be True by default, also the destination variable should be called dest="local_mods" for reflecting this.

Mario, Does that makes sense for you? If does, please modify the code accordingly.

review: Needs Fixing

[lp:~mariosplivalo/juju-deployer/juju-deployer]

118. By Mario Splivalo on 2014-05-29

Changed the name of the option to avoid confusion

119. By Mario Splivalo on 2014-05-30

Reverted option name to --local-mods, otherwise backwards compatibility might be broken

[Mario Splivalo (mariosplivalo)]

> The original option is semantically confusing, if you pass --local-mods it
> should means: enable local modifications and it should be True by default,
> also the destination variable should be called dest="local_mods" for
> reflecting this.
>
> Mario, Does that makes sense for you? If does, please modify the code
> accordingly.

Actually, the code is ok, just the help text is missleading. The default is for the deployer not to allow local charm modifications. If you, however, add --local-mods option, it will allow local charm modifications.
Therefore, the help text should read like this:

 -L, --local-mods Allow deployment of locally-modified charms

I've pushed the changes and created merge request.

[Kapil Thangavelu (hazmat)]

thanks.

On Fri, May 30, 2014 at 10:12 AM, Mario Splivalo <
<email address hidden>> wrote:

> > The original option is semantically confusing, if you pass --local-mods
> it
> > should means: enable local modifications and it should be True by
> default,
> > also the destination variable should be called dest="local_mods" for
> > reflecting this.
> >
> > Mario, Does that makes sense for you? If does, please modify the code
> > accordingly.
>
> Actually, the code is ok, just the help text is missleading. The default
> is for the deployer not to allow local charm modifications. If you,
> however, add --local-mods option, it will allow local charm modifications.
> Therefore, the help text should read like this:
>
> -L, --local-mods Allow deployment of locally-modified charms
>
>
> I've pushed the changes and created merge request.
> --
>
> https://code.launchpad.net/~mariosplivalo/juju-deployer/juju-deployer/+merge/221457
> Your team juju-deployers is subscribed to branch lp:juju-deployer.
>

=== modified file 'deployer/cli.py'
--- deployer/cli.py	2014-02-22 23:11:02 +0000
+++ deployer/cli.py	2014-05-30 14:05:01 +0000
@@ -34,7 +34,7 @@
         action="store_true", default=False)
     parser.add_argument(
         '-L', '--local-mods',
-        help='Disallow deployment of locally-modified charms',
+        help='Allow deployment of locally-modified charms',
         dest="no_local_mods", default=True, action='store_false')
     parser.add_argument(
         '-u', '--update-charms',