| Message ID | 20240715093047.49321-9-karthik.188@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | clang-format: add more rules and enable on CI | expand |
Karthik Nayak <karthik.188@gmail.com> writes: > A more ideal solution would be if 'clang-format' allowed us to append > rules to the existing '.clang-format' when invoked. But such an option > does not exist. Since modifying the in-tree '.clang-format' is only done > on the CI job for style-check and doesn't affect any other jobs and is > not persisted in any ways, this hack should be okay. I think our mails crossed, but I do not know why this hack is OK. Are there other CI jobs that muck with tracked files in the working tree? Thanks.
Junio C Hamano <gitster@pobox.com> writes: > Karthik Nayak <karthik.188@gmail.com> writes: > >> A more ideal solution would be if 'clang-format' allowed us to append >> rules to the existing '.clang-format' when invoked. But such an option >> does not exist. Since modifying the in-tree '.clang-format' is only done >> on the CI job for style-check and doesn't affect any other jobs and is >> not persisted in any ways, this hack should be okay. > > I think our mails crossed, but I do not know why this hack is OK. > Are there other CI jobs that muck with tracked files in the working > tree? > > Thanks. I mean from an operation of a CI job, the repository is discarded after the job. So there isn't a problem with murking with the working tree. But I agree to your latest response [1], it would be best to do this with a temporary file outside the working tree. Will send a new version soon! Thanks [1]: https://lore.kernel.org/r/xmqqle224npf.fsf@gitster.g
diff --git a/ci/run-style-check.sh b/ci/run-style-check.sh index 76dd37d22b..16b593fa1b 100755 --- a/ci/run-style-check.sh +++ b/ci/run-style-check.sh @@ -5,4 +5,17 @@ baseCommit=$1 +# 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 +echo "RemoveBracesLLVM: true" >>.clang-format + git clang-format --style file --diff --extensions c,h "$baseCommit"
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. A more ideal solution would be if 'clang-format' allowed us to append rules to the existing '.clang-format' when invoked. But such an option does not exist. Since modifying the in-tree '.clang-format' is only done on the CI job for style-check and doesn't affect any other jobs and is not persisted in any ways, this hack should be okay. [1]: https://clang.llvm.org/docs/ClangFormatStyleOptions.html#removebracesllvm Signed-off-by: Karthik Nayak <karthik.188@gmail.com> --- ci/run-style-check.sh | 13 +++++++++++++ 1 file changed, 13 insertions(+)