[PATCH] update autogen-waf.sh
Rowland Penny
rpenny at samba.org
Fri Aug 11 10:21:00 UTC 2017
On Fri, 11 Aug 2017 18:53:16 +1000
Martin Schwenke <martin at meltin.net> wrote:
> Hi Rowland,
>
> On Thu, 10 Aug 2017 14:21:34 +0100, Rowland Penny via
> samba-technical <samba-technical at lists.samba.org> wrote:
>
> > OK, here is the first one, this is basically just running the script
> > through 'shellcheck' and fixing any errors found, it should not
> > change how the script works.
>
> It all looks good. Can I suggest a minor improvement in a few places?
>
> Instead of doing:
>
> "$p"/Makefile
>
> it is probably better to do:
>
> "$p/Makefile"
>
> That way the quotes are around the whole string and future edits are
> less likely to be fragile.
>
> However, you need to be careful with examples like this:
>
> "$p"/include/config*.h*
>
> If you double-quote the wildcards then they don't get expanded. There
> are a few answers to this, including:
>
> "$p/include/config"*.h*
>
> Deciding where to end the quotes is quite subjective. :-)
>
> Personally, I tend to "over-punctuate" my shell scripts. I would
> write the first example as:
>
> "${p}/Makefile"
I understand all this Martin and as I said earlier, where do you stop ?
>
> While the braces aren't necessary in this case, I believe that they
> make the variable expansion more obvious... especially in cases where
> there are multiple expansions in a string. The only place I don't use
> the braces is when the variable expansion is the whole string, as in
> "$p".
I normally use them in every case, just in case ;-) but in this case,
I am just following 'shellcheck' and fixing obvious errors.
I think I will start again, follow 'shellcheck' but add what I would
normally do and also fix any errors. I will also take on board what you
say about the ctdb tests and ignore them.
Thanks
Rowland
More information about the samba-technical
mailing list