Skip to content
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

Open
annevk opened this issue May 18, 2018 · 12 comments
Open

Align bash scripts with Google style guide #155

annevk opened this issue May 18, 2018 · 12 comments

Comments

@annevk
Copy link
Member

annevk commented May 18, 2018

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.

@annevk
Copy link
Member Author

annevk commented May 18, 2018

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?

cc @sideshowbarker @domenic @foolip

@foolip
Copy link
Member

foolip commented May 18, 2018

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.

@annevk
Copy link
Member Author

annevk commented May 18, 2018

Functions have more options:

  • Preceded by function.
  • Followed by () (maybe mutually exclusive with the above?)
  • Underscore-separated or lower-camelCase.

@annevk
Copy link
Member Author

annevk commented May 18, 2018

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?

@annevk
Copy link
Member Author

annevk commented May 18, 2018

(I avoided a bunch of the duplication by thinking a little more btw, but there's still bits worth considering here I think.)

@sideshowbarker
Copy link
Contributor

sideshowbarker commented May 21, 2018

What we should do first is align on a coding style.

https://google.github.io/styleguide/shell.xml is the best available style guide I’ve come across.
https://www.chromium.org/chromium-os/shell-style-guidelines is a useful supplement to it.

I propose we adopt those as our style guides.


Q: Preferences on two-vs-four spaces? (Mild preference for two.)

https://google.github.io/styleguide/shell.xml#Indentation

Indent 2 spaces. No tabs.
Use blank lines between blocks to improve readability. Indentation is two spaces. Whatever you do, don't use tabs. For existing files, stay faithful to the existing indentation.


Q: Preferences on how we format functions? Preceded by function? Followed by () (maybe mutually exclusive with the above)? Underscore-separated or lower-camelCase?

https://google.github.io/styleguide/shell.xml#Function_Names

Lower-case, with underscores to separate words. …. Parentheses are required after the function name. The keyword function is optional, but must be used consistently throughout a project.
If you're writing single functions, use lowercase and separate words with underscore. … Braces must be on the same line as the function name … and no space between the function name and the parenthesis.
The function keyword is extraneous when "()" is present after the function name, but enhances quick identification of functions.


I'd go with the coding style that is the default in any tooling that's able to lint for bash coding style.

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.

@annevk annevk changed the title Deduplication of bash scripts Align bash scripts with Google style guide May 22, 2018
@annevk
Copy link
Member Author

annevk commented May 22, 2018

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.

@alrra
Copy link
Member

alrra commented May 22, 2018

lint for bash coding style

Maybe look into using https://github.com/koalaman/shellcheck?

@sideshowbarker
Copy link
Contributor

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)

@alrra
Copy link
Member

alrra commented May 22, 2018

@sideshowbarker Thanks for the info (and sorry for missing that).

@sideshowbarker
Copy link
Contributor

#165 seems to at least partially address this

@domenic
Copy link
Member

domenic commented Aug 1, 2018

One big still-missing thing is using local variables more/at all. This makes more sense now that things are more isolated to functions.

domenic added a commit that referenced this issue Oct 26, 2023
Mostly, by using more local variables. Helps with #155.
domenic added a commit that referenced this issue Oct 26, 2023
Mostly, by using more local variables. Helps with #155.
domenic added a commit that referenced this issue Oct 26, 2023
Mostly, by using more local variables. Helps with #155.
domenic added a commit that referenced this issue Oct 27, 2023
Mostly, by using more local variables. Helps with #155.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

5 participants