search

zmkfirmware / zmk

ZMK Firmware Repository
https://zmk.dev/
MIT License
2.63k stars 2.69k forks source link

Coding style #142

Closed okke-formsma closed 4 years ago

okke-formsma commented 4 years ago

I'd like to propose to use the zephyr coding style, which is the kernel coding style

In general, follow the Linux kernel coding style, with the following exceptions:

Add braces to every if, else, do, while, for and switch body, even for single-line code blocks. Use the --ignore BRACES flag to make checkpatch stop complaining.

Use spaces instead of tabs to align comments after declarations, as needed.

Use C89-style single line comments, / /. The C99-style single line comment, //, is not allowed.

Use /* / for doxygen comments that need to appear in the documentation.

CrossR commented 4 years ago

It looks like that also includes a formatter, which we get for free since its in ${ZMK_ROOT}/zephyr/scripts/ on mac/Linux. A bit more of a pain on Windows, but if we have a CI step to report errors they can always commit -> check -> format based on the results. (You can also technically set up GitHub Actions to run stuff on PRs and commit changes for you if you trust the in-place fix arg, but for a random script like this I'm not sure I'd trust it).

Assuming a style (any style) is picked, there is then a second discussion of if you should make all code compliant in a big fix-up commit, or fix files as you go etc. I personally lean on the "do a giant commit and get it over with" path, especially now that more recent Git versions let you ignore certain commits so you can filter it out so git blame etc all work sensibly and aren't destroyed.

BrainWart commented 4 years ago

I personally lean on the "do a giant commit and get it over with" path,

I agree. Makes it easier to write code following the conversions too because it's around already. Even if we just did the in-place fix, tuning the changes could be done later/incrementally if we wanted.

okke-formsma commented 4 years ago

uncrustify --replace --no-backup -l C -c ../zephyr/.uncrustify.cfg src/behaviors/behavior_tap_hold.c worked like a charm for me. I don't care if we do a big change right now or not, but let's at least make all new additions crust-free!

https://docs.zephyrproject.org/latest/contribute/index.html#uncrustify

petejohanson commented 4 years ago

I would actually make an alternate suggestion, and suggest clang-format based approach instead. There's tons more integrations and support for that instead of uncrustify. It should be easy for us to automate checks for PRs using it as well.

Thoughts?