Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 15 Oct 2012 19:26:22 +0100
From:      Chris Rees <crees@FreeBSD.org>
To:        Michael Telahun Makonnen <mmakonnen@gmail.com>
Cc:        freebsd-doc@freebsd.org, freebsd-rc@freebsd.org, freebsd-bugs@freebsd.org, "bug-followup@freebsd.org" <bug-followup@freebsd.org>
Subject:   Re: docs/172692: [PATCH] Bring parts of the rc scripting guides up to date
Message-ID:  <CADLo83_GZwcDaRc-Aujjrky7j0UdXkoD4c%2Bvg16FH2d%2Bd_LixQ@mail.gmail.com>
In-Reply-To: <507C39D0.9030909@gmail.com>
References:  <201210141319.q9EDJN6H085443@freefall.freebsd.org> <507C39D0.9030909@gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On 15/10/2012, Michael Telahun Makonnen <mmakonnen@gmail.com> wrote:
> Some minor nits:
>
> -       have the same prefix: <envar>${name}</envar>.  For
> +        have the same prefix: <envar>${name}_</envar>.  For
>
> You should add the same correction to the very next sentence, which reads:
>
> Note: While it is possible to use a shorter name internally, e.g.,  just

Done, thanks!

>
>
> @@ -491,15 +491,6 @@
>              our script will save us from possible
>            collisions with the &man.rc.subr.8; namespace.</para>
>
> -   <para>As long as an &man.rc.conf.5; variable and its
> -          internal equivalent are the same, we can use a more
> -           compact expression to set the default value:</para>
> -
> -         <programlisting>: ${dummy_msg:="Nothing
> started."}</programlisting>
> -
> -   <para>The current style is to use the more verbose form
> -           though.</para>
> -
>
> Not sure why you felt this paragraph needed to be removed.

Because the style is clearer and makes it much more obvious when a
variable is having its own default value set.  In ports at least, the
current style is to use the less verbose form.

> @@ -512,7 +503,11 @@
>
>         <callout arearefs="rcng-confdummy-msg">
>      <para>Here we use <envar>dummy_msg</envar> to actually
> -        control our script, i.e., to emit a variable message.</para>
> +    control our script, i.e., to emit a variable message.
> +         Use of a shell function is overkill here, since it only
> +       runs a single command; an equally valid alternative is:</para>
> +
> +      <programlisting>start_cmd="echo \"$dummy_msg\""</programlisting>
>
> While you are technically correct, I think you misunderstood the
> writer's intent, which was to show how an rc.conf(8) variable can be
> used in a subroutine to control the behavior of the command.  I agree
> that the example isn't a very good one (in that it doesn't depict a
> valid use case), but I think the "spirit" is correct.  Maybe you can
> suggest a better example?
>

I think that this script is very simple by design, and making a better
example would complicate it.  It is definitely worth pointing out the
alternative though; it makes useful food for thought; both examples
with a disclaimer.

Thank you very much for commenting, and I feel bad that I'm fobbing
you off.  You've raised genuine points, but I had already considered
them.  Please keep on at me if you think I still need to do something
about it.

Chris

[1] http://www.bayofrum.net/~crees/patches/rc-scripting-modernise2.diff

-- 
Chris Rees          | FreeBSD Developer
crees@FreeBSD.org   | http://people.freebsd.org/~crees



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CADLo83_GZwcDaRc-Aujjrky7j0UdXkoD4c%2Bvg16FH2d%2Bd_LixQ>