Re: [PATCH v6 6/6] ci/style-check: add `RemoveBracesLLVM` in CI job
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jun 19, 2025, 15:16 UTC
- Message-ID
- <xmqq5xgrdan8.fsf@gitster.g>
- In-Reply-To
- <20240723082111.874382-7-karthik.188@gmail.com>
Karthik Nayak <karthik.188@gmail.com> writes:
Show 30 quoted lines
> For 'clang-format', setting 'RemoveBracesLLVM' to 'true', adds a check > to ensure we avoid curly braces for single-statement bodies in > conditional blocks. > > However, the option does come with two warnings [1]: > > 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. While we want to experiment with the > rule, adding it to the in-tree '.clang-format' could affect end-users. > Let's only add it to the CI jobs for now. With time, we can evaluate > its efficacy and decide if we want to add it to '.clang-format' or > retract it entirely. We do so, by adding the existing rules in > '.clang-format' and this rule to a temp file outside the working tree, > which is then used by 'git clang-format'. This ensures we don't murk > with files in-tree. > > [1]: https://clang.llvm.org/docs/ClangFormatStyleOptions.html#removebracesllvm > > Signed-off-by: Karthik Nayak <karthik.188@gmail.com> > --- > ci/run-style-check.sh | 19 ++++++++++++++++++- > 1 file changed, 18 insertions(+), 1 deletion(-)
Have we done enough experiment by now and don't we want to move it to the set of rules in our tree?
How often do people use "git clang-format" before they submit their series these days, by the way?
Show 27 quoted lines
> diff --git a/ci/run-style-check.sh b/ci/run-style-check.sh
> index 76dd37d22b..6cd4b1d934 100755
> --- a/ci/run-style-check.sh
> +++ b/ci/run-style-check.sh
> @@ -5,4 +5,21 @@
>
> baseCommit=$1
>
> -git clang-format --style file --diff --extensions c,h "$baseCommit"
> +# 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 but keeps braces if one side of
> +# if/else if/.../else cascade has multi-statement body.
> +#
> +# As this rule comes with a warning [1], we want to experiment with it
> +# before adding it in-tree. since the CI job for the style check is allowed
> +# to fail, appending the rule here allows us to validate its efficacy.
> +# While also ensuring that end-users are not affected directly.
> +#
> +# [1]: https://clang.llvm.org/docs/ClangFormatStyleOptions.html#removebracesllvm
> +{
> + cat .clang-format
> + echo "RemoveBracesLLVM: true"
> +} >/tmp/clang-format-rules
> +
> +git clang-format --style=file:/tmp/clang-format-rules \
> + --diff --extensions c,h "$baseCommit"