launchpad-reviewers team mailing list archive

[Gavin Panella <gavin.panella@xxxxxxxxxxxxx>]

> A few notes from first reading:
> 
> 15      +wait_for_next() {
> 
> Better comment that, or at least include the next _what_ this waits for.

I've changed it to "wait_for_user_to_press_n".

> +cat <<'EOF'
> +
> +This script assumes that `vdenv` has been started, and that `zimmer` is
> +running. It will then *destroy* your development/demo database, rebuild
> +everything, and reconcile with Cobbler.
> 
> Did you test this in its current form?

Yes :)

> The shell will attempt backtick substitution on this text!  It will
> try to execute vdenv and zimmer.  Not what you want.

The single quotes around the opening EOF cause bash to not attempt
substitution on the content. The backticks will remain as they are.

> In fact, why did you use backticks at all?  They look weird, and the only
> reason for using them that I can see is that you really, really, *really*
> dislike double quotes and as long as you never, ever use them, and manage to
> avoid all apostrophes, it's usually (but not now) safe to use backticks as
> quotes inside a string.
> 
> That strikes me as a relatively weak reason to use backticks.

I've used backticks because I've grown accostomed to seeing them
denote code-like things, or non-prose, in the Launchpad code-base, and
when writing rST.

> +cat <<'EOF'
> +
> +Next the MAAS server will be started, at which point you can Juju.
> +
> +However, once MAAS is running, Juju needs to be pointed at it, and given
> +credentials with which to manipulate it:
> +
> +  1. Copy an OAuth token from <http://localhost:5240/account/prefs/>.
> +
> +  2. Interpolate it into the following:
> +
> +     {{{
> +     juju: environments
> +     environments:
> +       maas:
> +         type: maas
> +         maas-server: 'http://localhost:5240'
> +         maas-oauth: '${OAUTH_TOKEN_FROM_STEP_1}
> +         admin-secret: 'nothing'
> +         juju-origin: lp:maas
> +     }}}
> 
> 
> The shell will attempt variable substitution in this text, so
> '${OAUTH_TOKEN_FROM_STEP_1}' is likely to come out as the empty string.  Plus,
> the closing quote is missing.

This is safe for the same reason as backticks are safe. Thanks for
spotting the missing quote!

> 73      +  3. Customize `juju-origin` as desired.
> 74      +
> 75      +  4. Put it into `~/.juju/environments.yaml`.
> 
> Executing a few more unexpected commands here.  Not good.

And here :)

-- 
https://code.launchpad.net/~allenap/maas/pserv-cobbler-reconcile/+merge/98702
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/maas/pserv-cobbler-reconcile into lp:maas.