Fix .clang-format #1177
Fix .clang-format #1177
Conversation
WalkthroughAdds a prioritized include-category for Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
include/etl/chrono.h (1)
70-71: Add a brief rationale to deter “cleanup” removals laterA one-liner helps future maintainers understand why this block must stay unsorted.
-// clang-format off +// clang-format off +// Intentional: preserve exact order of private chrono includes..clang-format (1)
13-16: Harden platform.h category regex to cover <...> and subpathsCatches angle-bracket includes and nested paths, and anchors the match.
- - Regex: '("|/)platform\.h"$' + - Regex: '^(<|")(.*/)?platform\.h(>|")$' Priority: 1 - Regex: '.*' Priority: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.clang-format(1 hunks)include/etl/chrono.h(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: Windows - No STL - Force C++03
- GitHub Check: Windows - STL - Force C++03
- GitHub Check: Clang C++20 OSX - No STL (macos-13)
- GitHub Check: Clang C++20 OSX - STL - Force C++03 (macos-13)
- GitHub Check: Syntax Check - Clang C++14 Linux No STL (ubuntu-22.04)
- GitHub Check: Clang C++20 OSX - STL (macos-13)
- GitHub Check: Windows - No STL
- GitHub Check: GCC C++11 Linux - No STL (ubuntu-22.04)
- GitHub Check: Clang C++14 Linux - No STL (ubuntu-22.04)
- GitHub Check: GCC C++23 Linux - No STL - Force C++03 (ubuntu-22.04)
- GitHub Check: GCC C++23 Linux - STL (ubuntu-22.04)
- GitHub Check: GCC C++11 Linux - STL (ubuntu-22.04)
- GitHub Check: Clang C++23 OSX - STL (macos-13)
- GitHub Check: Clang C++23 OSX - No STL (macos-13)
- GitHub Check: Clang C++20 Linux - No STL (ubuntu-22.04)
- GitHub Check: Clang C++11 Linux - No STL (ubuntu-22.04)
- GitHub Check: Clang C++11 Linux - STL (ubuntu-22.04)
- GitHub Check: Clang C++23 Linux - STL - Force C++03 (ubuntu-22.04)
- GitHub Check: Clang C++17 Linux - STL (ubuntu-22.04)
- GitHub Check: Clang C++17 Linux - No STL (ubuntu-22.04)
🔇 Additional comments (2)
include/etl/chrono.h (1)
70-87: Correct use of clang-format region guards to preserve include orderThe // clang-format off/on markers are valid for clang-format v12 and will keep this include block untouched. (releases.llvm.org)
.clang-format (1)
10-12: Re-enabling include sorting with preserved blocks aligns with v12 behaviorIncludeBlocks: Preserve + SortIncludes: true is the right combo to sort within blocks while keeping block boundaries. (releases.llvm.org)
rolandreichweinbmw
force-pushed
the
fix-clang-format
branch
from
ef2aa26 to
71ee192
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.clang-format (2)
12-16: Avoid flattening system vs project groups—add explicit categories.Defining only two categories (platform.h, then catch‑all) overrides Chromium defaults and can intermix and "project" headers within the same block. If you want classic grouping while still prioritizing platform.h, extend the categories as below.
IncludeCategories: - - Regex: '^(<|")(.*/)?platform\.h(>|")$' - Priority: 1 - - Regex: '.*' - Priority: 2 + - Regex: '^(<|")(.*/)?platform\.h(>|")$' + Priority: 1 # project-wide platform gate first + - Regex: '^<.*>$' + Priority: 2 # system/STL headers + - Regex: '^".*"$' + Priority: 3 # project/third-party quoted headers + - Regex: '.*' + Priority: 4 # anything unexpected
13-14: Narrow the platform.h regex if third‑party headers share that name.Current pattern matches any path ending in platform.h. If dependencies also provide a platform.h, they’ll be hoisted too. If the intent is only ETL’s header, narrow the path.
- - Regex: '^(<|")(.*/)?platform\.h(>|")$' + - Regex: '^(<|")(.*/)?etl/platform\.h(>|")$'
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.clang-format(1 hunks)include/etl/chrono.h(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- include/etl/chrono.h
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: Clang C++20 Linux - STL - Force C++03 (ubuntu-22.04)
- GitHub Check: Clang C++20 Linux - No STL - Force C++03 (ubuntu-22.04)
- GitHub Check: Clang C++20 Linux - No STL (ubuntu-22.04)
- GitHub Check: Clang C++23 Linux - No STL - Force C++03 (ubuntu-22.04)
- GitHub Check: GCC C++17 Linux - No STL (ubuntu-22.04)
- GitHub Check: GCC C++20 Linux - No STL (ubuntu-22.04)
- GitHub Check: GCC C++20 Linux - No STL - Force C++03 (ubuntu-22.04)
- GitHub Check: GCC C++17 Linux - STL (ubuntu-22.04)
- GitHub Check: Clang C++17 Linux - No STL (ubuntu-22.04)
- GitHub Check: GCC C++20 Linux - STL (ubuntu-22.04)
- GitHub Check: Clang C++17 Linux - STL (ubuntu-22.04)
- GitHub Check: GCC C++20 Linux - STL - Force C++03 (ubuntu-22.04)
- GitHub Check: Clang C++11 Linux - No STL (ubuntu-22.04)
- GitHub Check: Clang C++23 Linux - STL - Force C++03 (ubuntu-22.04)
- GitHub Check: Windows - STL
- GitHub Check: GCC C++23 Linux - STL (ubuntu-22.04)
- GitHub Check: Clang C++23 Linux - No STL (ubuntu-22.04)
- GitHub Check: Clang C++11 Linux - STL (ubuntu-22.04)
- GitHub Check: GCC C++11 Linux - No STL (ubuntu-22.04)
- GitHub Check: GCC C++11 Linux - STL (ubuntu-22.04)
🔇 Additional comments (2)
.clang-format (2)
10-11: Re-enabling include sorting while preserving blocks looks good.
SortIncludes: truewithIncludeBlocks: Preserverestores deterministic ordering without merging developer-defined blocks.
10-16: Ensure clang-format v12+ is installed and configured
CI and developer environments currently lackclang-format, so the newIncludeCategoriescan’t be validated. Install clang-format v12 or later, then run:
clang-format --versionto confirm the versionclang-format -style=file -dump-configto verify theIncludeCategoriessettings- a dry-run (
clang-format -n --Werror --style=file) across your code to ensure no sensitive include sequences are reordered.
2 participants
With this change, the tests are successful after clang-format reformatting the whole ETL.
Summary by CodeRabbit
Style
Chores