[PATCH] update autogen-waf.sh
Martin Schwenke
martin at meltin.net
Fri Aug 11 10:32:11 UTC 2017
On Fri, 11 Aug 2017 11:21:00 +0100, Rowland Penny via samba-technical
<samba-technical at lists.samba.org> wrote:
> 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 ?
Cool. I'm just over-communicating given the usual time turnaround
for long distance email conversations... :-)
> > 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.
Yeah, shellcheck often provides hints that the code could be better.
> 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.
I think that's a good move.
That said, if you really do want to attack the CTDB tests then I think
the order could be:
1. Fix shellcheck hits in unit tests
2. Fix shellcheck hits in the (bash) integration tests
3. Convert bash to POSIX sh
However, it is a major undertaking...
TV time. :-)
peace & happiness,
martin
More information about the samba-technical
mailing list