RespiraWorks / Ventilator

Fully-featured ICU ventilator design, optimized for manufacture using commonly available components and free to license. Repository tracks all mechanical, electrical and systems design, software, requirements and regulatory documentation.
Apache License 2.0
127 stars 39 forks source link

Enforce consistent C++ naming conventions #117

Open jkff opened 4 years ago

jkff commented 4 years ago

What has to be done Currently our code uses a bunch of inconsistent naming conventions - there's a mixture of like_this, likeThis, LikeThis and Like_this for class/type names, functions, parameters, variables and constants.

We should enable build-time checks that enforce some useful set of naming conventions. Per personal communication with Martin, basing it on the Google style guide is okay.

This issue can be closed when ./test.sh includes running a configurable set of naming enforcements.

How do you know it has to be done

Code pointers

Most likely we should do this as just another clang-tidy check (https://github.com/RespiraWorks/VentilatorSoftware/issues/132)

martukas commented 4 years ago

I'm personally partial to the google style guide: CamelCase for classes and like_this for variables, etc.. I'm guessing you might be familiar with this convention? :)

jkff commented 4 years ago

Yeah, following Google style guide is a reasonable default choice and I am indeed familiar with it :)

For somebody who decides to take on this issue: it can be done incrementally e.g. in the following steps:

This would generate commits that are not quite as huge and tedious to author as one giant commit bringing all the code in compliance with everything.

Another approach, perhaps even better, is to first gradually bring things in compliance, and then add the tool enforcement + rules in the same commit with final fixups.

jkff commented 4 years ago

I'm gonna try to take this one and enable clang-tidy while we're at it, as clang-tidy supports enforcement of naming conventions too.

martukas commented 3 years ago

@jkff do you still intend to work on this or shall we untag you and keep this open for newbies or something?

jkff commented 3 years ago

Yeah unfortunately I don't have time work on RespiraWorks these days - untagged myself. All the best to y'all!

martukas commented 3 years ago

Just taking notes on the progress here. Looks like you have to sudo apt install clang-format I have tried this:

git clang-format --extensions cpp,hpp --style=google -v

but it seems to only fix whitespace and not variable naming conventions

dheater commented 3 years ago

Just taking notes on the progress here. Looks like you have to sudo apt install clang-format I have tried this:

git clang-format --extensions cpp,hpp --style=google -v

but it seems to only fix whitespace and not variable naming conventions

Apparently clang-tidy can enforce naming conventions. I've not used this feature yet, but the documentation is here: https://clang.llvm.org/extra/clang-tidy/checks/readability-identifier-naming.html

I don't see predefined style options (ex,--style=google) for clang-tidy.

asmodai27 commented 3 years ago

Looks like this is what we are looking for: https://gist.github.com/airglow923/1fa3bda42f2b193920d7f46ee8345e04