search

zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.37k stars 6.35k forks source link

doc: contribute guidelines: Explain when not to use clang-format #73465

Open Snevzor opened 3 months ago

Snevzor commented 3 months ago

Is your enhancement proposal related to a problem? Please describe. Running clang-format on a file with old code that doesn't follow the current coding style makes a substantial code change unreviewable, unless clang-format is performed beforehand and put in a separate commit that can be skipped faster by the reviewers. The change (new code) can now easily be reformatted by clang-format too before committing. With https://github.com/zephyrproject-rtos/zephyr/pull/73360 I experienced this is actually not desired.

Describe the solution you'd like Update https://docs.zephyrproject.org/latest/contribute/guidelines.html#clang-format to explain why reformatting old code before adding the changes is not desired and add what to do in this situation. The new code should adhere to the current coding style, even if it deviates from the style used in the c file?

Perhaps some small examples can be added.

cdwilson commented 3 months ago

The new code should adhere to the current coding style, even if it deviates from the style used in the c file?

FYI, I'm not sure that .clang-format actually defines the project coding style. From https://github.com/zephyrproject-rtos/zephyr/issues/52712#issuecomment-1333955406:

clang-format does not define project coding style, it is up to you to post-fix the code formatted by clang-format

I'd love to have some guidance/examples about how/when to use clang-format with Zephyr given that it conflicts with the formatting done by checkpatch.

cdwilson commented 3 months ago

BTW, this has been asked before and was closed (but wasn't clear if there was an objection to updating the docs to make this clarification).

Snevzor commented 3 months ago

Thanks for linking. It is confusing and ultimately leading to lots of wasted time and effort for both new unaware contributors but definitely the reviewers.