Junio C Hamano <gitster@xxxxxxxxx> writes: > Karthik Nayak <karthik.188@xxxxxxxxx> writes: > >> +# [1]: https://clang.llvm.org/docs/ClangFormatStyleOptions.html#removebracesllvm >> +echo "RemoveBracesLLVM: true" >> .clang-format >> + >> git clang-format --style file --diff --extensions c,h "$baseCommit" > > Style: lose the SP between the redirection operator and its operand. > Will change. > I know this is well intentioned, but will there be any downsides for > running the check always against a dirty working tree? > > I know during a CI run, the working tree will not be completely > clean, as we create and leave build artifacts, but this is as far as > I know the first instance of us munging a tracked path, changing the > working tree in a way that "git describe --dirty" would notice. > > This is done as the last (and only) step of check-style job and the > ci/run-style-check.sh script may not do anything like "git describe > --dirty" right now, but things can change. Perhaps I am worried > about this a bit too much. > Exactly my thoughts too. I did test on GitLab's CI and all other jobs were unaffected. So I think we're good here. --- After reading your mail, I figured I should also check GitHub's CI for this particular change and realized there is a slight issue with my current series. GitLab's CI highlights style check jobs which failed with a yellow warning symbol [1], even with the 'ignore failing check-style' directive. But GitHub's actions, simply marks the job as successful even if the job failed [1]. This was an oversight on my side, since I expected similar behavior. Seems like the required dependency wasn't even installed on GitHub causing 'git clang-format' to fail. Unfortunately this means all GitHub workflows for style-check will be green even if there were style issues found. I couldn't find a way to fix this from reading the documentation. This will not be an issue once we enforce, but till then users cannot rely on the job's outcome for the job's status in GitHub. They will have to see the logs to know if style check failed. I will re-roll with a fix, but will wait a day or so, to avoid spamming. > I unfortunately couldn't find an option to "git clang-format" to > tell it to read from an extra file in addition to the usual > ".clang-format" file---if such an option existed, we obviously could > use an untracked/ignored file to prepare the custom format file and > use it without making the working tree dirty. > This was also something I looked for, but couldn't find. I should have added that to the commit message. Will do so in the reroll. > So perhaps the posted change, while making us feel dirty, is the > best we could do for this step. > > Thanks. Yes, I think it is okay. I'm hoping we can move it in-tree someday. [1]: https://gitlab.com/gitlab-org/git/-/pipelines/1372326813 [2]: https://github.com/KarthikNayak/git/actions/runs/9921272597/job/27409047062
Attachment:
signature.asc
Description: PGP signature
