-
Notifications
You must be signed in to change notification settings - Fork 62
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Align bash scripts with Google style guide #155
Comments
What we should do first is align on a coding style. Preferences on two-vs-four spaces? (Mild preference for two.) Preferences on how we format functions? |
I'd go with the coding style that is the default in any tooling that's able to lint for bash coding style. Failing that I like two spaces and { on the same line. |
Functions have more options:
|
Also, https://stackoverflow.com/questions/4542732/how-do-i-negate-a-test-with-regular-expressions-in-a-bash-script recommends using lowercase/camelcase variable names for non-environment variables. Seems worth considering? |
(I avoided a bunch of the duplication by thinking a little more btw, but there's still bits worth considering here I think.) |
https://google.github.io/styleguide/shell.xml is the best available style guide I’ve come across. I propose we adopt those as our style guides.
https://google.github.io/styleguide/shell.xml#Indentation
https://google.github.io/styleguide/shell.xml#Function_Names
I’ve not managed to find any good tooling that does that. At least not anything that’s widely used. I did find https://trevormccasland.wordpress.com/2017/09/28/bashate-a-style-checker-for-bash-scripts/ but the behavior of that doesn’t seem to be based on any common style guide — and conflicts with the Google Shell Style Guide (e.g., it looks for 4-space indents rather than 2-space indents). There’s no similar accompanying tooling for the Google Shell Style Guide, as far as I know. |
I guess we won't follow all their recommendations (e.g., file names), but it seems reasonable to adopt the ones we're currently inconsistent on. |
Maybe look into using https://github.com/koalaman/shellcheck? |
Yeah we actually are already using shellcheck. So the discussion in this issue about an additional level of checking — more style checking (as opposed to general linting) |
@sideshowbarker Thanks for the info (and sorry for missing that). |
#165 seems to at least partially address this |
One big still-missing thing is using local variables more/at all. This makes more sense now that things are more isolated to functions. |
Mostly, by using more local variables. Helps with #155.
Mostly, by using more local variables. Helps with #155.
Mostly, by using more local variables. Helps with #155.
Mostly, by using more local variables. Helps with #155.
Per https://stackoverflow.com/questions/12815774/importing-functions-from-a-shell-script this seems to be doable. Not sure it's worth it quite yet though since we'd also have to fetch the script from another repository.
The text was updated successfully, but these errors were encountered: