git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 0/8] clang-format: add more rules and enable on CI

From
Karthik Nayak <karthik.188@gmail.com>
Date
Jul 11, 2024, 08:30 UTC
Message-ID
<20240711083043.1732288-1-karthik.188@gmail.com>
In-Reply-To
<20240708092317.267915-1-karthik.188@gmail.com>
This is v2 of my series to add clang-format to the CI.

The series was mostly inspired by a patch sent recently to 'include kh_foreach* macros in ForEachMacros' [1]. I was wondering why we don't run the formatting on CI and reduce some of the workload of reviewers.

We have a '.clang-format' file to provide rules for code formatting. The commits 1-6 aims to add more rules to the file while deprecating old ones.

Commit 7 enables CI action on GitHub and GitLab to also check for the code formatting. Currently, it is allowed to fail. This way we can build confidence and fine tune the values as needed and finally enforce the check in a future patch. I'm not well versed with GitHub workflows, and I mostly tried to copy existing work there. Expecting some feedback in that section!

Commit 8 fixes an existing issue with the 'check-whitespace' job, which is failing as success in the GitLab CI.

1: https://lore.kernel.org/git/4e7893f5-2dd9-46cf-8a64-cf780f4e1730@web.de/
Changes against v1:
* Fixed a lot of typos.
* Added more details about the warnings specified by clang-format for
'RemoveBracesLLVM' in commit 5. Added more details too.
* The last two commits were modified to use "CI_MERGE_REQUEST_TARGET_BRANCH_SHA"
and fallback to "CI_MERGE_REQUEST_DIFF_BASE_SHA" if the former is not
available in GitLab's CI.
* Use `!/bin/sh` for the run-style-check script.
* Modified the last commit to remove block around the && check in the
whitespace script.
* Use `git rev-parse --verify --quiet` to verify the base commit.
Karthik Nayak (8):
  clang-format: indent preprocessor directives after hash
  clang-format: avoid spacing around bitfield colon
  clang-format: ensure files end with newlines
  clang-format: replace deprecated option with 'SpacesInParens'
  clang-format: avoid braces on simple single-statement bodies
  clang-format: formalize some of the spacing rules
  ci: run style check on GitHub and GitLab
  check-whitespace: detect if no base_commit is provided
 .clang-format                     | 42 +++++++++++++++++++++++++++----
 .github/workflows/check-style.yml | 29 +++++++++++++++++++++
 .gitlab-ci.yml                    | 24 +++++++++++++++++-
 ci/check-whitespace.sh            | 10 ++++++--
 ci/install-dependencies.sh        |  2 +-
 ci/run-style-check.sh             |  8 ++++++
 6 files changed, 106 insertions(+), 9 deletions(-)
 create mode 100644 .github/workflows/check-style.yml
 create mode 100755 ci/run-style-check.sh
Range-diff against v1:
1:  6cf91ffc86 = 1:  6cf91ffc86 clang-format: indent preprocessor directives after hash
2:  beb002885f = 2:  beb002885f clang-format: avoid spacing around bitfield colon
3:  3031be43e7 = 3:  3031be43e7 clang-format: ensure files end with newlines
4:  bc1550e300 = 4:  bc1550e300 clang-format: replace deprecated option with 'SpacesInParens'
5:  cb6097aa22 ! 5:  840318a4c9 clang-format: avoid braces on simple single-statement bodies
    @@ Commit message
         Set the 'RemoveBracesLLVM' to 'true' which ensures that we avoid curly
         braces for single-statement bodies in conditional blocks.
     
    +    This option does come with two warnings:
    +
    +        This option will be renamed and expanded to support other styles.
    +
    +    and
    +
    +        Setting this option to true could lead to incorrect code formatting
    +        due to clang-format’s lack of complete semantic information. As
    +        such, extra care should be taken to review code changes made by
    +        this option.
    +
    +    The latter seems to be of concern. But since we only use clang-format to
    +    verify the format and not to apply formatting, we should be okay here.
    +
         Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
     
      ## .clang-format ##
    @@ .clang-format: IndentWrappedFunctionNames: false
      PointerAlignment: Right
      
     +# Remove optional braces of control statements (if, else, for, and while)
    -+# according to the LLVM coding style
    -+# This avoids braces on simple single-statement bodies of statements.
    ++# according to the LLVM coding style. This avoids braces on simple
    ++# single-statement bodies of statements but keeps braces if one side of
    ++# if/else if/.../else cascade has multi-statement body.
     +RemoveBracesLLVM: true
     +
      # Don't insert a space after a cast
6:  c4b6e1e82f ! 6:  0ecfd8d24b clang-format: formalize some of the spacing rules
    @@ Commit message
         clang-format: formalize some of the spacing rules
     
         There are some spacing rules that we follow in the project and it makes
    -    sen to formalize them:
    +    sense to formalize them:
         * Ensure there is no space inserted after the logical not '!' operator.
    -    * Ensure there is no space before the case statement's color.
    +    * Ensure there is no space before the case statement's colon.
         * Ensure there is no space before the first bracket '[' of an array.
         * Ensure there is no space in empty blocks.
     
7:  37f1e5c702 ! 7:  11a9ac4935 ci: run style check on GitHub and GitLab
    @@ Commit message
         this job to fail, so we can validate if this is useful and eventually
         enforce it.
     
    +    For GitLab, we use the 'CI_MERGE_REQUEST_TARGET_BRANCH_SHA' variable by
    +    default to obtain the base SHA of the merged pipeline (which is only
    +    available for merged pipelines [1]). Otherwise we use the
    +    'CI_MERGE_REQUEST_DIFF_BASE_SHA' variable.
    +
    +    [1]: https://docs.gitlab.com/ee/ci/variables/predefined_variables.html#predefined-variables-for-merge-request-pipelines
    +
         Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
     
      ## .github/workflows/check-style.yml (new) ##
    @@ .gitlab-ci.yml: check-whitespace:
     +  before_script:
     +    - ./ci/install-dependencies.sh
     +  script:
    -+    - ./ci/run-style-check.sh "$CI_MERGE_REQUEST_DIFF_BASE_SHA"
    ++    - |
    ++      if [ -z ${CI_MERGE_REQUEST_TARGET_BRANCH_SHA} ]; then
    ++        ./ci/run-style-check.sh "$CI_MERGE_REQUEST_DIFF_BASE_SHA"
    ++      else
    ++        ./ci/run-style-check.sh "$CI_MERGE_REQUEST_TARGET_BRANCH_SHA"
    ++      fi
     +  rules:
     +    - if: $CI_PIPELINE_SOURCE == 'merge_request_event'
     +
    @@ ci/install-dependencies.sh: ubuntu-*)
     
      ## ci/run-style-check.sh (new) ##
     @@
    -+#!/usr/bin/env sh
    ++#!/bin/sh
     +#
     +# Perform style check
     +#
8:  c6d07402af < -:  ---------- check-whitespace: detect if no base_commit is provided
-:  ---------- > 8:  caccbf8264 check-whitespace: detect if no base_commit is provided
-- 
2.45.1
Previous: Chris TorekNext: Karthik Nayak
Message 52 of 127 in “clang-format: add more rules and enable on CI”
  1. 0/8 clang-format: add more rules and enable on CIKarthik Nayak, Jul 8, 2024
  2. 1/8 clang-format: indent preprocessor directives after hashKarthik Nayak, Jul 8, 2024
  3. Chris TorekJul 8, 2024
  4. Junio C HamanoJul 8, 2024
  5. Karthik NayakJul 8, 2024
  6. Re* [PATCH 1/8] clang-format: indent preprocessor directives after hashJunio C Hamano, Jul 8, 2024
  7. 2/8 clang-format: avoid spacing around bitfield colonKarthik Nayak, Jul 8, 2024
  8. 3/8 clang-format: ensure files end with newlinesKarthik Nayak, Jul 8, 2024
  9. Junio C HamanoJul 8, 2024
  10. 4/8 clang-format: replace deprecated option with 'SpacesInParens'Karthik Nayak, Jul 8, 2024
  11. Junio C HamanoJul 8, 2024
  12. 5/8 clang-format: avoid braces on simple single-statement bodiesKarthik Nayak, Jul 8, 2024
  13. Junio C HamanoJul 8, 2024
  14. Karthik NayakJul 8, 2024
  15. Phillip WoodJul 12, 2024
  16. Karthik NayakJul 13, 2024
  17. phillip.wood123@gmail.comJul 13, 2024
  18. Karthik NayakJul 13, 2024
  19. 6/8 clang-format: formalize some of the spacing rulesKarthik Nayak, Jul 8, 2024
  20. Junio C HamanoJul 8, 2024
  21. Karthik NayakJul 8, 2024
  22. Justin ToblerJul 8, 2024
  23. Karthik NayakJul 8, 2024
  24. Eric SunshineJul 8, 2024
  25. Karthik NayakJul 10, 2024
  26. 7/8 ci: run style check on GitHub and GitLabKarthik Nayak, Jul 8, 2024
  27. Junio C HamanoJul 8, 2024
  28. Karthik NayakJul 8, 2024
  29. Re* [PATCH 7/8] ci: run style check on GitHub and GitLabJunio C Hamano, Jul 8, 2024
  30. Justin ToblerJul 8, 2024
  31. brian m. carlsonJul 9, 2024
  32. Justin ToblerJul 8, 2024
  33. Karthik NayakJul 8, 2024
  34. Justin ToblerJul 9, 2024
  35. Karthik NayakJul 9, 2024
  36. Justin ToblerJul 9, 2024
  37. Karthik NayakJul 10, 2024
  38. 8/8 check-whitespace: detect if no base_commit is providedKarthik Nayak, Jul 8, 2024
  39. Chris TorekJul 8, 2024
  40. Georg PfuetzenreuterJul 8, 2024
  41. Chris TorekJul 8, 2024
  42. Karthik NayakJul 8, 2024
  43. Junio C HamanoJul 8, 2024
  44. Junio C HamanoJul 8, 2024
  45. Karthik NayakJul 10, 2024
  46. Junio C HamanoJul 10, 2024
  47. Karthik NayakJul 11, 2024
  48. Junio C HamanoJul 11, 2024
  49. Justin ToblerJul 8, 2024
  50. Karthik NayakJul 10, 2024
  51. Chris TorekJul 8, 2024
  52. 0/8 clang-format: add more rules and enable on CIKarthik Nayak, Jul 11, 2024
  53. 1/8 clang-format: indent preprocessor directives after hashKarthik Nayak, Jul 11, 2024
  54. 2/8 clang-format: avoid spacing around bitfield colonKarthik Nayak, Jul 11, 2024
  55. 3/8 clang-format: ensure files end with newlinesKarthik Nayak, Jul 11, 2024
  56. 4/8 clang-format: replace deprecated option with 'SpacesInParens'Karthik Nayak, Jul 11, 2024
  57. 5/8 clang-format: avoid braces on simple single-statement bodiesKarthik Nayak, Jul 11, 2024
  58. Junio C HamanoJul 11, 2024
  59. Karthik NayakJul 12, 2024
  60. Junio C HamanoJul 12, 2024
  61. Phillip WoodJul 12, 2024
  62. 6/8 clang-format: formalize some of the spacing rulesKarthik Nayak, Jul 11, 2024
  63. 8/8 check-whitespace: detect if no base_commit is providedKarthik Nayak, Jul 11, 2024
  64. Justin ToblerJul 11, 2024
  65. Junio C HamanoJul 11, 2024
  66. Karthik NayakJul 12, 2024
  67. Junio C HamanoJul 12, 2024
  68. 7/8 ci: run style check on GitHub and GitLabKarthik Nayak, Jul 11, 2024
  69. Junio C HamanoJul 11, 2024
  70. 0/8 clang-format: add more rules and enable on CIKarthik Nayak, Jul 13, 2024
  71. 1/8 clang-format: indent preprocessor directives after hashKarthik Nayak, Jul 13, 2024
  72. 2/8 clang-format: avoid spacing around bitfield colonKarthik Nayak, Jul 13, 2024
  73. 3/8 clang-format: ensure files end with newlinesKarthik Nayak, Jul 13, 2024
  74. 4/8 clang-format: replace deprecated option with 'SpacesInParens'Karthik Nayak, Jul 13, 2024
  75. 5/8 clang-format: formalize some of the spacing rulesKarthik Nayak, Jul 13, 2024
  76. 6/8 ci: run style check on GitHub and GitLabKarthik Nayak, Jul 13, 2024
  77. Junio C HamanoJul 13, 2024
  78. 7/8 check-whitespace: detect if no base_commit is providedKarthik Nayak, Jul 13, 2024
  79. 8/8 ci/style-check: add `RemoveBracesLLVM` to '.clang-format'Karthik Nayak, Jul 13, 2024
  80. Junio C HamanoJul 13, 2024
  81. Karthik NayakJul 13, 2024
  82. Ramsay JonesJul 13, 2024
  83. Karthik NayakJul 15, 2024
  84. Junio C HamanoJul 15, 2024
  85. Karthik NayakJul 15, 2024
  86. Junio C HamanoJul 15, 2024
  87. Karthik NayakJul 15, 2024
  88. Junio C HamanoJul 15, 2024
  89. Karthik NayakJul 18, 2024
  90. Junio C HamanoJul 18, 2024
  91. 0/8 clang-format: add more rules and enable on CIKarthik Nayak, Jul 15, 2024
  92. 1/8 clang-format: indent preprocessor directives after hashKarthik Nayak, Jul 15, 2024
  93. 2/8 clang-format: avoid spacing around bitfield colonKarthik Nayak, Jul 15, 2024
  94. 3/8 clang-format: ensure files end with newlinesKarthik Nayak, Jul 15, 2024
  95. 4/8 clang-format: replace deprecated option with 'SpacesInParens'Karthik Nayak, Jul 15, 2024
  96. 5/8 clang-format: formalize some of the spacing rulesKarthik Nayak, Jul 15, 2024
  97. 6/8 ci: run style check on GitHub and GitLabKarthik Nayak, Jul 15, 2024
  98. 7/8 check-whitespace: detect if no base_commit is providedKarthik Nayak, Jul 15, 2024
  99. 8/8 ci/style-check: add `RemoveBracesLLVM` to '.clang-format'Karthik Nayak, Jul 15, 2024
  100. Junio C HamanoJul 15, 2024
  101. Karthik NayakJul 15, 2024
  102. 0/6 : add more rules and enable on CIKarthik Nayak, Jul 18, 2024
  103. 1/6 clang-format: indent preprocessor directives after hashKarthik Nayak, Jul 18, 2024
  104. 2/6 clang-format: avoid spacing around bitfield colonKarthik Nayak, Jul 18, 2024
  105. 3/6 clang-format: formalize some of the spacing rulesKarthik Nayak, Jul 18, 2024
  106. 4/6 ci: run style check on GitHub and GitLabKarthik Nayak, Jul 18, 2024
  107. Junio C HamanoJul 18, 2024
  108. Junio C HamanoJul 18, 2024
  109. 5/6 check-whitespace: detect if no base_commit is providedKarthik Nayak, Jul 18, 2024
  110. Junio C HamanoJul 18, 2024
  111. Karthik NayakJul 19, 2024
  112. Junio C HamanoJul 19, 2024
  113. Karthik NayakJul 22, 2024
  114. 6/6 ci/style-check: add `RemoveBracesLLVM` in CI jobKarthik Nayak, Jul 18, 2024
  115. Junio C HamanoJul 18, 2024
  116. 0/6 clang-format: add more rules and enable on CIKarthik Nayak, Jul 23, 2024
  117. 1/6 clang-format: indent preprocessor directives after hashKarthik Nayak, Jul 23, 2024
  118. Patrick SteinhardtJul 24, 2024
  119. Karthik NayakJul 24, 2024
  120. 2/6 clang-format: avoid spacing around bitfield colonKarthik Nayak, Jul 23, 2024
  121. 3/6 clang-format: formalize some of the spacing rulesKarthik Nayak, Jul 23, 2024
  122. 4/6 ci: run style check on GitHub and GitLabKarthik Nayak, Jul 23, 2024
  123. 5/6 check-whitespace: detect if no base_commit is providedKarthik Nayak, Jul 23, 2024
  124. 6/6 ci/style-check: add `RemoveBracesLLVM` in CI jobKarthik Nayak, Jul 23, 2024
  125. Junio C HamanoJun 19, 2025
  126. Karthik NayakJun 19, 2025
  127. Junio C HamanoJun 20, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.