-
Notifications
You must be signed in to change notification settings - Fork 374
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
Automatic code formatting #574
Comments
I guess we definitely need something like this. 👍 About tools to use and format rules, I let the team decide what best fits. |
I am really in favor of picking a standard like clang-format as suggested, as I do not see any real advantage in relation to the effort to maintain our own standard (or deviation from an existing standard). |
I'm in favor of adopting a code standard. No strong opinions on the specifics, what is suggested above works for me. Before any decision is made we should get input from @sbertin-telular also. |
I'm also in favor of a code standard with no strong opinions on the specifics. If it can easily be automated and enforced even better. |
The er-coap-13 is exempt as we might want to get upstream changes merged at some point.
Used command: find coap core data examples include tests -type f -not \ -path 'examples/shared/tinydtls/*' \( -name '*.c' -or -name '*.h' \) \ -exec clang-format-10 --style=file -i "{}" \;
All future cosmetic only commits should be appended to this list. This allows "git blame" to ignore them: git config blame.ignoreRevsFile .git-blame-ignore-revs
The er-coap-13 is exempt as we might want to get upstream changes merged at some point.
Used command: find coap core data examples include tests -type f -not \ -path 'examples/shared/tinydtls/*' \( -name '*.c' -or -name '*.h' \) \ -exec clang-format-10 --style=file -i "{}" \;
All future cosmetic only commits should be appended to this list. This allows "git blame" to ignore them: git config blame.ignoreRevsFile .git-blame-ignore-revs
The er-coap-13 is exempt as we might want to get upstream changes merged at some point.
This ensures that all code being submitted adheres to the format dictated by clang-format. Please note: - The er-coap-13 code gets exempt, as at some point, we probably want to to interact with whatever upstream there might be. For those files, just do what existing code does (which varies every few lines). - Default is clang-format-10, which is available in the Ubuntu 20.04 GitHub runner.
This ensures that all code being submitted adheres to the format dictated by clang-format. Please note: - The er-coap-13 code gets exempt, as at some point, we probably want to to interact with whatever upstream there might be. For those files, just do what existing code does (which varies every few lines). - Default is clang-format-10, which is available in the Ubuntu 20.04 GitHub runner.
This ensures that all code being submitted adheres to the format dictated by clang-format. Please note: - The er-coap-13 code gets exempt, as at some point, we probably want to to interact with whatever upstream there might be. For those files, just do what existing code does (which varies every few lines). - Default is clang-format-10, which is available in the Ubuntu 20.04 GitHub runner.
git-clang-format ensures that all code being submitted adheres to the format dictated by clang-format. Please note: - The er-coap-13 code gets exempt, as at some point, we probably want to to interact with whatever upstream there might be. For those files, just do what existing code does (which varies every few lines). - Default is clang-format-10, which is available in the Ubuntu 20.04 GitHub runner.
git-clang-format ensures that all code being submitted adheres to the format dictated by clang-format. Please note: - The er-coap-13 code gets exempt, as at some point, we probably want to to interact with whatever upstream there might be. For those files, just do what existing code does (which varies every few lines). - Default is clang-format-10, which is available in the Ubuntu 20.04 GitHub runner.
git-clang-format ensures that all code being submitted adheres to the format dictated by clang-format. Please note: - The er-coap-13 code gets exempt, as at some point, we probably want to to interact with whatever upstream there might be. For those files, just do what existing code does (which varies every few lines). - Default is clang-format-10, which is available in the Ubuntu 20.04 GitHub runner.
git-clang-format ensures that all code being submitted adheres to the format dictated by clang-format. The code style is based on LLVM (the creator of clang-format), but with 4 instead of 2 spaces indent width and a maximum of 120 instead of 80 characters per line. Please note: - The er-coap-13 code gets exempt, as at some point, we probably want to to interact with whatever upstream there might be. For those files, just do what existing code does (which varies every few lines). - Default is clang-format-10, which is available in the Ubuntu 20.04 GitHub runner.
I tried different clang-format style definitions to approximate the style being used most often. However, always ended up with man thousands of line changes. And while using --ignore-revs-file would work for Git itself, many 3rd-party-tools (like SonarQube) do not support it, blaming me for all the issues... :/ Therefore, I implemented a partial solution for this issue: #595 It enforces the code style on new code, which is far less intrusive than formatting all existing code. Would like to get some feedback on it. |
git-clang-format ensures that all *new* code being submitted adheres to the format dictated by clang-format. The code style is based on LLVM (the creator of clang-format), but with 4 instead of 2 spaces indent width and a maximum of 120 instead of 80 characters per line. Please note: - The er-coap-13 code gets exempt, as at some point, we probably want to to interact with whatever upstream there might be. For those files, just do what existing code does (which varies every few lines). - Default is clang-format-10, which is available in the Ubuntu 20.04 GitHub runner.
git-clang-format ensures that all *new* code being submitted adheres to the format dictated by clang-format. The code style is based on LLVM (the creator of clang-format), but with 4 instead of 2 spaces indent width and a maximum of 120 instead of 80 characters per line in order to blend well with existing code. Please note: - The er-coap-13 code gets exempt, as at some point, we probably want to to interact with whatever upstream there might be. For those files, just do what existing code does (which varies every few lines). - Default is clang-format-10, which is available in the Ubuntu 20.04 GitHub runner.
git-clang-format ensures that all *new* code being submitted adheres to the format dictated by clang-format. The code style is based on LLVM (the creator of clang-format), but with 4 instead of 2 spaces indent width and a maximum of 120 instead of 80 characters per line in order to blend in well with existing code. Please note: - The er-coap-13 code gets exempt, as at some point, we probably want to to interact with whatever upstream there might be. For those files, just do what existing code does (which varies every few lines). - Default is clang-format-10, which is available in the Ubuntu 20.04 GitHub runner.
git-clang-format ensures that all *new* code being submitted adheres to the format dictated by clang-format. The code style is based on LLVM (the creator of clang-format), but with 4 instead of 2 spaces indent width and a maximum of 120 instead of 80 characters per line in order to blend in well with existing code. Please note: - The er-coap-13 code gets exempt, as at some point, we probably want to to interact with whatever upstream there might be. For those files, just do what existing code does (which varies every few lines). - Default is clang-format-10, which is available in the Ubuntu 20.04 GitHub runner.
I used the following script: for f in $(find . -type f \( -name '*.c' -or -name '*.h' \) \ -not -path '*/examples/shared/tinydtls/*' \ -not -path '*/coap/er-coap-13/*'); do end="$(grep --line-number --extended-regexp "\\*{70}/" "${f}" | awk -F: '{print $1}')" echo "Formatting ${f}, line 1 to ${end}" clang-format-10 -i --lines="1:${end}" "${f}" done
The last commit was purely cosmetic. Therefore, adding it to .git-blame-ignore-revs.
git-clang-format ensures that all *new* code being submitted adheres to the format dictated by clang-format. The code style is based on LLVM (the creator of clang-format), but with 4 instead of 2 spaces indent width and a maximum of 120 instead of 80 characters per line in order to blend in well with existing code. Please note: - The er-coap-13 code gets exempt, as at some point, we probably want to to interact with whatever upstream there might be. For those files, just do what existing code does (which varies every few lines). - Default is clang-format-10, which is available in the Ubuntu 20.04 GitHub runner.
git-clang-format ensures that all *new* code being submitted adheres to the format dictated by clang-format. The code style is based on LLVM (the creator of clang-format), but with 4 instead of 2 spaces indent width and a maximum of 120 instead of 80 characters per line in order to blend in well with existing code. Please note: - The er-coap-13 code gets exempt, as at some point, we probably want to to interact with whatever upstream there might be. For those files, just do what existing code does (which varies every few lines). - Default is clang-format-10, which is available in the Ubuntu 20.04 GitHub runner.
I used the following script: for f in $(find . -type f \( -name '*.c' -or -name '*.h' \) \ -not -path '*/examples/shared/tinydtls/*' \ -not -path '*/coap/er-coap-13/*'); do end="$(grep --line-number --extended-regexp "\\*{70}/" "${f}" | awk -F: '{print $1}')" echo "Formatting ${f}, line 1 to ${end}" clang-format-10 -i --lines="1:${end}" "${f}" done
The last commit was purely cosmetic. Therefore, adding it to .git-blame-ignore-revs.
With #595 merged, only the CMake formatting capabilities are missing. |
This is in reference to #602 (comment). It seems off topic there. A pure cosmetic patch which reformats just the portion of code changed creates a burden on the developer. They need to make their changes to find out what needs reformatting, reformat, commit, and re-apply their changes. Repeat if more changes are requested in the pull request. Meanwhile unchanged code doesn't get the formatting updated. This could lead to a real mess. I'd rather see the code reformatted entirely (maybe 1 file per pull request to be more manageable) and move forward from there. |
cmake-format ensures that our CMake code adheres to the format dictated by cmake-format. The code style is based on default one, with some minor adjustments.
cmake-format ensures that our CMake code adheres to the format dictated by cmake-format. The code style is based on default one, with some minor adjustments.
cmake-format ensures that our CMake code adheres to the format dictated by cmake-format. The code style is based on default one, with some minor adjustments.
cmake-format ensures that our CMake code adheres to the format dictated by cmake-format. The code style is based on default one, with some minor adjustments.
cmake-format ensures that our CMake code adheres to the format accepted by cmake-format. The code style is based on default one, with some minor adjustments.
cmake-format ensures that our CMake code adheres to the format accepted by cmake-format. The code style is based on default one, with some minor adjustments.
cmake-format ensures that our CMake code adheres to the format accepted by cmake-format. The code style is based on default one, with some minor adjustments.
cmake-format ensures that our CMake code adheres to the format accepted by cmake-format. The code style is based on default one, with some minor adjustments.
cmake-format ensures that our CMake code adheres to the format accepted by cmake-format. The code style is based on default one, with some minor adjustments.
Took a stab at the remaining linting/formatting of CMake code in ##653. Since most of the CMake code got changed for #653, I went on to reformat all files, and therefore it will be possible to just run cmake-format on all CMake files like this: git ls-files '*CMakeLists.txt' '*.cmake' | xargs cmake-lint
@sbertin-telular This will not happen for the CMake files. |
cmake-format ensures that our CMake code adheres to the format accepted by cmake-format. The code style is based on default one, with some minor adjustments.
cmake-format ensures that our CMake code adheres to the format accepted by cmake-format. The code style is based on default one, with some minor adjustments.
cmake-format ensures that our CMake code adheres to the format accepted by cmake-format. The code style is based on default one, with some minor adjustments.
cmake-format ensures that our CMake code adheres to the format accepted by cmake-format. The code style is based on default one, with some minor adjustments.
cmake-format ensures that our CMake code adheres to the format accepted by cmake-format. The code style is based on default one, with some minor adjustments.
This ensures we can always run cmake-format on our CMake files.
This ensures we can always run cmake-format on our CMake files.
This ensures we can always run cmake-format on our CMake files.
With #655 merged, this should be done for good. If the partial formatting of C code turns out to be too much of an issue, please reopen or create a new issue. |
The current method (require new code do be properly formatted) actually requires (cosmetic) changes quite far apart from the actual code change. This either results in (too) noisy commits (a) or, when striving for separation of functional and cosmetic changes, requires a follow-up commit (b): (a) makes it hard to reason about and reviews contributions and (b) is not something we can seriously ask from any contributors IMHO. Therefore, I can also to the conclusion that reformatting the entire code base is the way to go. I will create a PR shortly. |
Historically, the Wakaama code base did not use any kind of code formatting tools. Therefore, every file looks slightly different, making it more hard than necessary to read the code, figuring out and sticking to the predominant style.
Therefore, for C code I propose to pick clang-format (version 10 for now), using a code style close to the one LLVM, the creators of clang-format, use. Foreseeable adaptions might be:
For CMake files, https://github.com/cheshirekow/cmake_format seems to be the tool to pick.
Acceptance criteria:
Code and comments checked for being garbled, exceptions in place where sensibledid not format the whole code basePlease note:
Any opinions on this?
The text was updated successfully, but these errors were encountered: