-
Notifications
You must be signed in to change notification settings - Fork 613
ci: use clang-format from apt llvm #1042
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Most of C++ developers prefer Unix box, so we should use the Unix for style check. 1. The lint time decrease from 1m 47s to 28s which is an improvement of 73.83%. 2. Upgrade clang-format is easier with one parameter. 3. llvm apt support Debian and Ubuntu distribution. 4. Improve CI waiting time for developer.
only clang-format-18 is required, it would be faster like bellow(22.04 jammy for example) wget https://apt.llvm.org/llvm-snapshot.gpg.key
sudo gpg --dearmor < llvm-snapshot.gpg.key > /usr/share/keyrings/llvm-archive-keyring.gpg
echo "deb [signed-by=/usr/share/keyrings/llvm-archive-keyring.gpg] http://apt.llvm.org/jammy/ llvm-toolchain-jammy-18 main" | sudo tee /etc/apt/sources.list.d/llvm-toolchain-jammy-18.list
sudo apt-get update
sudo apt-get install -y clang-format-18
sudo ln -sf /usr/bin/clang-format-18 /usr/local/bin/clang-format
clang-format --version |
Yeah, in the initial draft I want to install clang-format package too. The current lint time 27s in one task[1] which is faster enough for us, IMO. The current script is robust and maitained by LLVM team which support all debian derivation distributions and all clang version. |
安裝步驟也太複雜了。 畢竟只是個輔助性的開發工具,我覺得沒必要做複雜的優化。 |
The current clang-format 18 and 19 on Ubunut 24.04 has issue with GitHub runner image. The lint failed with weird error on [1] with clang-format-18 and on [2] with clang-format-19. On my local machine box with openSUSE, both clang-format 18 and 19 are fine. The clang-format 18 and 19 on both llvm, openSUSE and latest Ubuntu image with Podman are fine. If we want use We need to fix one format style with clang-format-20 which will break old distribution with older clang-format. So I picked the llvm apt at final, fix for the weird clang format error under GitHub runner iamge are welcome. |
Only one style violation on clang-format-20 under openSUSE.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, LGTM
Pull request
Issue tracker
Fixes will automatically close the related issue
Most of C++ developers prefer Unix box, so we should use the Unix for style check.
Feature
Describe feature of pull request
Unit test
Manual test
Code Review
Additional Info