whatwg / html-build

Build scripts for https://github.com/whatwg/html
Other
65 stars 62 forks source link

Align bash scripts with Google style guide #155

Open annevk opened 6 years ago

annevk commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

Functions have more options:

annevk commented 6 years ago

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 commented 6 years ago

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

sideshowbarker commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

lint for bash coding style

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

sideshowbarker commented 6 years ago

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 commented 6 years ago

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

sideshowbarker commented 6 years ago

165 seems to at least partially address this

domenic commented 6 years ago

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