On 13/04/2019 13:31, Jeremy Huntwork wrote:
> On Sat, Apr 13, 2019 at 04:16 Pierre Labastie <[email protected]
> <mailto:[email protected]>> wrote:
> 
>     On 13/04/2019 00:18, Jeremy Huntwork wrote:
>     > On Fri, Apr 12, 2019 at 6:12 PM Jeremy Huntwork
>     > <[email protected]
>     <mailto:[email protected]>> wrote:
>     >>
>     >> You're right, that would work just as well. I think I just get used to
>     >> switching from 'ls' to 'find' in scripts. I'll send an updated patch.
>     >>
>     >
> 
>     Thanks. Apart from one thing (BLUE is used in one function, and should be
>     declared), I think your patch is OK. But there is something I do not like:
>     shellcheck propose to "export" variables that look unused. But if they are
>     used in a function, they do not need to be exported, and exporting too 
> many
>     variables may be a problem: for example, if you tick "Run Makefile", the 
> whole
>     build process may be run with the exported variables (depending on sudo's
>     "keep_env" setting), and who knows which kind of clash may result? So I'd
>     rather use a shellcheck directive to disable the shellcheck warning when 
> I am
>     sure the variable is used in a function. Or otherwise make the exported
>     variable names unique (like adding a prefix such as JH_).
> 
>     I may be missing something here, so I'm open to discussion. But if you 
> agree,
>     I'll disable shellcheck SC2034 rather than exporting variables.
> 
>     Pierre
> 
> 
> 
> I agree, I tend to prefer prefixed variables. Also, I think shellcheck
> recommends exporting because those variables are included in other sourced
> files that it can’t verify directly. To ignore the suggestion for these cases
> should be fine. 
> 
> I’ll send another patch in a little while, or if you want to just go ahead
> with your adjustments that’s fine too. 
> 

Let me finish with this patch. I've managed to eliminate all the shellcheck
warnings, without exporting variables. I've added a couple of double
backslash's to silent the last ones (*). I'm running a build right now, to
check that everything is OK.

Do you want to check the other scripts? Nothing mandatory, of course. I'll be
glad to keep collaborating with you.

Pierre
(*): I'd never realized that:
$ echo -e "\n"
and:
$ echo -e "\\n"
were equivalent (print a blank line), while:
$ echo -e \n
and:
$ echo -e \\n
aren't (the first one prints a "n", the second one prints a blank line). I
begin to understand the use of "printf" builtin
-- 
http://lists.linuxfromscratch.org/listinfo/alfs-discuss
FAQ: http://www.linuxfromscratch.org/faq/
Unsubscribe: See the above information page

Reply via email to