chore: Google Shell Style Compliance #326
Conversation
mohitsaxenaknoldus
requested review from
MaxymVlasov and
yermulnik
as code owners
mohitsaxenaknoldus
force-pushed
the
issue324
branch
from
72ceef4 to
d8455f9
Compare
hooks/infracost_breakdown.sh
Outdated
| local -r hook_config="$1" | ||
| local args | ||
| read -r -a args <<< "$2" | ||
| read -r -a args <<<"$2" |
There was a problem hiding this comment.
The whitespace reduction changes is what shellcheck GH action refuses to accept.
There was a problem hiding this comment.
Would you please pet linter by reverting such whitespace change along with fix other linter errors and warnings.
| @@ -6,7 +6,7 @@ set -eo pipefail | |||
| # Arguments: | |||
| # script_dir - absolute path to hook dir location | |||
| ####################################################################### | |||
| function common::initialize { | |||
| common::initialize() { | |||
There was a problem hiding this comment.
The name() to function name change was an effort by @MaxymVlasov
Please negotiate this with Max.
There was a problem hiding this comment.
Yeah, please return function word. Google style guide do not determine which syntax should be used
About () - not sure is it needed. It looks like apendix, during bash not be able to achieve function function_name(var1, var2)
There was a problem hiding this comment.
Ah, George mentined that () will not works within function
#326 (comment)
| @@ -6,7 +6,7 @@ set -eo pipefail | |||
| # Arguments: | |||
| # script_dir - absolute path to hook dir location | |||
| ####################################################################### | |||
| function common::initialize { | |||
| common::initialize() { | |||
There was a problem hiding this comment.
Yeah, please return function word. Google style guide do not determine which syntax should be used
About () - not sure is it needed. It looks like apendix, during bash not be able to achieve function function_name(var1, var2)
| ####################################### | ||
| # main function | ||
| # Globals: | ||
| # ARGS | ||
| # HOOK_CONFIG | ||
| # SCRIPT_DIR | ||
| # Arguments: | ||
| # None | ||
| ####################################### |
There was a problem hiding this comment.
Too obvious. Not needed
Put an
xinto the box if that apply:Description of your changes
This PR only contains small non-breaking changes. I'll create another PR which will require some careful testing.
#324
How can we test changes