mbox series

[v6,0/6] clang-format: add more rules and enable on CI

Message ID 20240723082111.874382-1-karthik.188@gmail.com (mailing list archive)
Headers show
Series clang-format: add more rules and enable on CI | expand

Message

karthik nayak July 23, 2024, 8:21 a.m. UTC
This is v6 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-3 aims to add more rules to the file.

Commit 4 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 5 fixes an existing issue with the 'check-whitespace' job, which
is failing as success in the GitLab CI.

Commit 6 adds the `RemoveBracesLLVM` only in the context of the CI. If we
decide against it, we could drop this commit from the series.

1: https://lore.kernel.org/git/4e7893f5-2dd9-46cf-8a64-cf780f4e1730@web.de/

Changes from v5:
- Used 'C=${A-${B:?}}' instead of 'C=${A-${B?}}' for obtaining the base OID
in GitLab CI script. The difference being that the former also checks
for B being empty. With this we can simplify the script from

    R=${A-${B?}}
    if test -z "$R"
    then
        error
    fi

to 
    R=${A-${B:?}} || exit


* CI without issues
  - GitLab: https://gitlab.com/gitlab-org/git/-/jobs/7394162598
  - GitHub: https://github.com/KarthikNayak/git/actions/runs/10037207688/job/27736512636?pr=1
* CI with issues detected
  - GitLab: https://gitlab.com/gitlab-org/git/-/jobs/7394406605
  - GitHub: https://github.com/KarthikNayak/git/actions/runs/10037229508/job/27736859486

Karthik Nayak (6):
  clang-format: indent preprocessor directives after hash
  clang-format: avoid spacing around bitfield colon
  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
  ci/style-check: add `RemoveBracesLLVM` in CI job

 .clang-format                     | 25 +++++++++++++++++++++++
 .github/workflows/check-style.yml | 34 +++++++++++++++++++++++++++++++
 .gitlab-ci.yml                    | 25 ++++++++++++++++++++++-
 ci/check-whitespace.sh            | 10 +++++++--
 ci/install-dependencies.sh        |  4 ++++
 ci/run-style-check.sh             | 25 +++++++++++++++++++++++
 6 files changed, 120 insertions(+), 3 deletions(-)
 create mode 100644 .github/workflows/check-style.yml
 create mode 100755 ci/run-style-check.sh

Range-diff against v5:
1:  6cf91ffc86 = 1:  6cf91ffc86 clang-format: indent preprocessor directives after hash
2:  beb002885f = 2:  beb002885f clang-format: avoid spacing around bitfield colon
3:  3922529001 = 3:  3922529001 clang-format: formalize some of the spacing rules
4:  ae23eb5af8 ! 4:  14718625ad ci: run style check on GitHub and GitLab
    @@ .gitlab-ci.yml: check-whitespace:
     +  # be defined in all pipelines.
     +  script:
     +    - |
    -+      R=${CI_MERGE_REQUEST_TARGET_BRANCH_SHA-${CI_MERGE_REQUEST_DIFF_BASE_SHA?}}
    -+
    -+      if test -z "$R"
    -+      then
    -+        echo "CI_MERGE_REQUEST_DIFF_BASE_SHA should always exist!"
    -+        exit 1
    -+      fi
    ++      R=${CI_MERGE_REQUEST_TARGET_BRANCH_SHA-${CI_MERGE_REQUEST_DIFF_BASE_SHA:?}} || exit
     +      ./ci/run-style-check.sh "$R"
     +  rules:
     +    - if: $CI_PIPELINE_SOURCE == 'merge_request_event'
5:  a38cde03a8 ! 5:  fb8c30852b check-whitespace: detect if no base_commit is provided
    @@ .gitlab-ci.yml: check-whitespace:
        script:
     -    - ./ci/check-whitespace.sh "$CI_MERGE_REQUEST_TARGET_BRANCH_SHA"
     +    - |
    -+      R=${CI_MERGE_REQUEST_TARGET_BRANCH_SHA-${CI_MERGE_REQUEST_DIFF_BASE_SHA?}}
    -+
    -+      if test -z "$R"
    -+      then
    -+        echo "CI_MERGE_REQUEST_DIFF_BASE_SHA should always exist!"
    -+        exit 1
    -+      fi
    ++      R=${CI_MERGE_REQUEST_TARGET_BRANCH_SHA-${CI_MERGE_REQUEST_DIFF_BASE_SHA:?}} || exit
     +      ./ci/check-whitespace.sh "$R"
        rules:
          - if: $CI_PIPELINE_SOURCE == 'merge_request_event'
6:  008e77bd0a = 6:  08c5bb777d ci/style-check: add `RemoveBracesLLVM` in CI job