Declare all local variables using bash local keyword

[msprotz]

By convention, lowercase variable names are local to each function, and ALLCAPS variables are script-global and do not need a new keyword.

[darrenge]

Thoughts on this naming convention: "The function keyword is extraneous when "()" is present after the function name, but enhances quick identification of functions." I kind of like the idea of putting "function" on each function as it does make it easier to find and identify functions.

[msprotz]

Yes, we're bailing out if we don't have bash 4.2, so we should leverage bash-isms as much as possible. Putting the opening brace on the same line is also something that only works in modern bash... we shouldn't hesitate to favor readability.

[darrenge]

All sounds good. I found the this standard here: https://google.github.io/styleguide/shell.xml#Naming_Conventions

[darrenge]

@protz -- what are you talking about here "do not need a new keyword". Also ... what are your thoughts about using "local" instead of implied local inside of function? From standards "Ensure that local variables are only seen inside a function and its children by using local when declaring them. This avoids polluting the global name space and inadvertently setting variables that may have significance outside the function"

[msprotz]

Sorry for the terse initial bug report. What I meant is in the title, i.e. declare all variables that are local to a function using the local keyword. What I meant in the body of the bug report is, the script already more or less follows the convention, so local variables should be easy to spot (they're lowercase) and one can easily add the local keyword in front of them. Global variables are uppercase, and do not need any special treatment (there is no global keyword).

Hope that helps!

By the way the google style guide is an excellent document, great find. I'm curious to see if there's improvements to our coding style we can make. I seem to remember someone adding a test somewhere of the form [ x"$foo" = x"blah" ]...

[darrenge]

Good ... that makes sense and what I was looking at.

One interesting thing from the standard: "Declaration and assignment must be separate statements when the assignment value is provided by a command substitution; as the 'local' builtin does not propagate the exit code from the command substitution." my_func2() { local name="$1"

-- Separate lines for declaration and assignment: local my_var my_var="$(my_func)" || return

-- DO NOT do this: $? contains the exit code of 'local', not my_func local my_var="$(my_func)" [[ $? -eq 0 ]] || return

[darrenge]

Couple other things: 1) If a variable is declared outside of a function, it is basically acting like a global, so I upper cased those 2) "no space between the function name and the parenthesis."

[msprotz]

1. Yes, excellent!
2. I don't have a strong opinion on 2. as long as we're consistent -- happy to have consistency all across one way or another.

[darrenge]

Closing out as the code has been merged