[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