-
Notifications
You must be signed in to change notification settings - Fork 164
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
Update to use the Google shell styleguide #268
Comments
Maybe the Linux kernel style (like using tabs etc.) should be followed since this is something related to it? Also, maybe the huge bash script could be split into multiple files for modularity... 80 char limitation will make the script code longer, so if the file is split, reading might become easier. Bash's |
I don't understand what you mean - kernel is written in C, so their styleguide follows that, while dkms is in shell/bash. Sounds like trying to push a square peg through a round hole, with this suggestion. Yes, dkms is big, but chunking it into smaller pieces mostly increases the odds of breaking things. Not unless we get an order of magnitude more tests. Nevertheless - that's not the concern raised here. |
In regard of: dell#268. Replaced all instances of `[ expr ]' I could find with `[[ expr ]]', or `(( expr ))'. Also removed the now obsolete quoting. Some unnecessary grouped conditional commands have been consolidated. Signed-off-by: Mart Frauenlob <[email protected]>
In regard of: dell#268. Replaced all instances of `[ expr ]' I could find with `[[ expr ]]', or `(( expr ))'. Also removed the now obsolete quoting. Some unnecessary grouped conditional commands have been consolidated. Signed-off-by: Mart Frauenlob <[email protected]>
In regard of: #268. Replaced all instances of `[ expr ]' I could find with `[[ expr ]]', or `(( expr ))'. Also removed the now obsolete quoting. Some unnecessary grouped conditional commands have been consolidated. Signed-off-by: Mart Frauenlob <[email protected]>
Due to its age (et al) dkms has various styles mixed in. Most prominently - it does not use safe(r) bash constructs line
[[
and((
,set -euo pipefail
, while in other cases the code is borderline bonkers.Please keep changes separated logically and do not clump everything into a mega-commit or two.
Ideally we'll also get a CI stage/action that ensures these are consistent, somewhat at least. Can be done alongside the proposed shellcheck #267 or via other means.
The text was updated successfully, but these errors were encountered: