dkms icon indicating copy to clipboard operation
dkms copied to clipboard

Update to use the Google shell styleguide

Open evelikov opened this issue 2 years ago • 2 comments

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 https://github.com/dell/dkms/issues/267 or via other means.

evelikov avatar Oct 26 '22 10:10 evelikov

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 source can "import" functions from other files.

siddhpant avatar Oct 27 '22 11:10 siddhpant

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.

evelikov avatar Oct 27 '22 11:10 evelikov