style(clang-format): Re-order header include priorities so external headers appear before the project's headers; Update to clang-format v21.1 and apply the corresponding format changes. #156
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…nal headers appear before the project's headers.
WalkthroughUpdated repository formatting and linting: .gitignore entries added; src/.clang-format include-category priorities adjusted; many headers reordered and duplicate fmt includes removed; minor formatting of type aliases; lint-requirements tightened; dev-utils submodule pointer advanced. No functional or API changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)📓 Common learnings🔇 Additional comments (1)
✨ 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: 3
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
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 (15)
.gitignore(1 hunks)src/.clang-format(1 hunks)src/log_surgeon/SchemaParser.cpp(2 hunks)src/log_surgeon/finite_automata/Dfa.hpp(1 hunks)src/log_surgeon/finite_automata/DfaState.hpp(1 hunks)src/log_surgeon/finite_automata/DfaTransition.hpp(1 hunks)src/log_surgeon/finite_automata/Nfa.hpp(1 hunks)src/log_surgeon/finite_automata/NfaSpontaneousTransition.hpp(1 hunks)src/log_surgeon/finite_automata/NfaState.hpp(1 hunks)src/log_surgeon/finite_automata/RegexAST.hpp(1 hunks)src/log_surgeon/finite_automata/RegisterOperation.hpp(1 hunks)src/log_surgeon/finite_automata/TagOperation.hpp(1 hunks)src/log_surgeon/wildcard_query_parser/QueryInterpretation.cpp(1 hunks)tests/test-schema.cpp(1 hunks)tools/yscope-dev-utils(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,h,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit configuration file
- Prefer
false == <expression>rather than!<expression>.
Files:
src/log_surgeon/finite_automata/NfaState.hppsrc/log_surgeon/finite_automata/NfaSpontaneousTransition.hpptests/test-schema.cppsrc/log_surgeon/finite_automata/Nfa.hppsrc/log_surgeon/wildcard_query_parser/QueryInterpretation.cppsrc/log_surgeon/finite_automata/RegisterOperation.hppsrc/log_surgeon/finite_automata/DfaState.hppsrc/log_surgeon/SchemaParser.cppsrc/log_surgeon/finite_automata/TagOperation.hppsrc/log_surgeon/finite_automata/RegexAST.hppsrc/log_surgeon/finite_automata/DfaTransition.hppsrc/log_surgeon/finite_automata/Dfa.hpp
🧠 Learnings (16)
📚 Learning: 2024-11-13T20:02:13.737Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#48
File: src/log_surgeon/finite_automata/RegexNFAState.hpp:0-0
Timestamp: 2024-11-13T20:02:13.737Z
Learning: In `src/log_surgeon/finite_automata/RegexNFAState.hpp`, the constructor `RegexNFAState(std::set<Tag const*> tags, RegexNFAState const* dest_state)` has been updated to use `std::vector<Tag const*> tags` instead of `std::set`.
Applied to files:
src/log_surgeon/finite_automata/NfaState.hppsrc/log_surgeon/finite_automata/NfaSpontaneousTransition.hpptests/test-schema.cppsrc/log_surgeon/finite_automata/Nfa.hppsrc/log_surgeon/finite_automata/DfaState.hppsrc/log_surgeon/SchemaParser.cppsrc/log_surgeon/finite_automata/TagOperation.hppsrc/log_surgeon/finite_automata/RegexAST.hppsrc/log_surgeon/finite_automata/DfaTransition.hppsrc/log_surgeon/finite_automata/Dfa.hpp
📚 Learning: 2024-10-24T15:54:35.193Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#42
File: src/log_surgeon/finite_automata/RegexNFA.hpp:442-456
Timestamp: 2024-10-24T15:54:35.193Z
Learning: In the C++ file `src/log_surgeon/finite_automata/RegexNFA.hpp`, for the `RegexNFA::serialize()` function, prioritize code clarity over efficiency when handling string operations.
Applied to files:
src/log_surgeon/finite_automata/NfaState.hpptests/test-schema.cppsrc/log_surgeon/finite_automata/Nfa.hppsrc/log_surgeon/finite_automata/DfaState.hppsrc/log_surgeon/SchemaParser.cppsrc/log_surgeon/finite_automata/RegexAST.hppsrc/log_surgeon/finite_automata/Dfa.hpp
📚 Learning: 2024-11-02T09:13:56.755Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#47
File: src/log_surgeon/finite_automata/RegexNFAState.hpp:127-128
Timestamp: 2024-11-02T09:13:56.755Z
Learning: `RegexNFAUTF8State` is defined as a type alias for `RegexNFAState<RegexNFAStateType::UTF8>`.
Applied to files:
src/log_surgeon/finite_automata/NfaState.hpptests/test-schema.cppsrc/log_surgeon/SchemaParser.cppsrc/log_surgeon/finite_automata/RegexAST.hpp
📚 Learning: 2024-10-24T15:54:19.228Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#42
File: src/log_surgeon/finite_automata/RegexNFA.hpp:99-105
Timestamp: 2024-10-24T15:54:19.228Z
Learning: In `src/log_surgeon/finite_automata/RegexNFA.hpp`, it's acceptable to have constructors without the `explicit` specifier. Do not suggest adding `explicit` to constructors in this file.
Applied to files:
src/log_surgeon/finite_automata/NfaState.hppsrc/log_surgeon/finite_automata/NfaSpontaneousTransition.hpptests/test-schema.cppsrc/log_surgeon/finite_automata/Nfa.hppsrc/log_surgeon/SchemaParser.cppsrc/log_surgeon/finite_automata/RegexAST.hpp
📚 Learning: 2024-11-18T16:45:46.073Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#50
File: src/log_surgeon/finite_automata/Tag.hpp:0-0
Timestamp: 2024-11-18T16:45:46.073Z
Learning: The class `TagPositions` was removed from `src/log_surgeon/finite_automata/Tag.hpp` as it is no longer needed.
Applied to files:
src/log_surgeon/finite_automata/NfaState.hppsrc/log_surgeon/finite_automata/NfaSpontaneousTransition.hppsrc/log_surgeon/finite_automata/Nfa.hppsrc/log_surgeon/finite_automata/RegisterOperation.hppsrc/log_surgeon/finite_automata/DfaState.hppsrc/log_surgeon/finite_automata/TagOperation.hppsrc/log_surgeon/finite_automata/RegexAST.hppsrc/log_surgeon/finite_automata/DfaTransition.hppsrc/log_surgeon/finite_automata/Dfa.hpp
📚 Learning: 2024-11-27T22:25:35.608Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#56
File: src/log_surgeon/finite_automata/RegisterHandler.hpp:0-0
Timestamp: 2024-11-27T22:25:35.608Z
Learning: In the `RegisterHandler` class in `src/log_surgeon/finite_automata/RegisterHandler.hpp`, the methods `add_register` and `append_position` rely on `emplace_back` and `m_prefix_tree.insert` to handle exceptions correctly and maintain consistent state without requiring additional exception handling.
Applied to files:
src/log_surgeon/finite_automata/NfaState.hppsrc/log_surgeon/finite_automata/Nfa.hppsrc/log_surgeon/finite_automata/RegisterOperation.hppsrc/log_surgeon/finite_automata/DfaState.hppsrc/log_surgeon/finite_automata/TagOperation.hppsrc/log_surgeon/finite_automata/RegexAST.hppsrc/log_surgeon/finite_automata/DfaTransition.hppsrc/log_surgeon/finite_automata/Dfa.hpp
📚 Learning: 2024-11-02T09:18:31.046Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#47
File: src/log_surgeon/finite_automata/TaggedTransition.hpp:16-37
Timestamp: 2024-11-02T09:18:31.046Z
Learning: In `src/log_surgeon/finite_automata/TaggedTransition.hpp`, the classes `PositiveTaggedTransition` and `NegativeTaggedTransition` currently do not share enough functionality to justify refactoring into a common base class.
Applied to files:
src/log_surgeon/finite_automata/NfaSpontaneousTransition.hppsrc/log_surgeon/finite_automata/TagOperation.hppsrc/log_surgeon/finite_automata/RegexAST.hppsrc/log_surgeon/finite_automata/Dfa.hpp
📚 Learning: 2024-11-13T22:25:54.168Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#48
File: tests/test-tag.cpp:10-10
Timestamp: 2024-11-13T22:25:54.168Z
Learning: In the log-surgeon codebase (C++), particularly in the finite automata components involving the `Tag` class (`src/log_surgeon/finite_automata/Tag.hpp`), it's important to ensure that `Tag*` pointers in other objects cannot be `nullptr`. Test cases should focus on validating that these `Tag*` pointers are not null where they are used, and handle `nullptr` appropriately.
Applied to files:
src/log_surgeon/finite_automata/NfaSpontaneousTransition.hppsrc/log_surgeon/finite_automata/Nfa.hppsrc/log_surgeon/finite_automata/TagOperation.hppsrc/log_surgeon/finite_automata/RegexAST.hpp
📚 Learning: 2025-08-08T10:23:06.281Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp:80-88
Timestamp: 2025-08-08T10:23:06.281Z
Learning: In y-scope/log-surgeon (C++), small function definitions are allowed to remain inline in headers. For src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp, do not suggest moving small methods like QueryInterpretation::append_variable_token to the .cpp for “consistency” in future reviews.
Applied to files:
src/log_surgeon/wildcard_query_parser/QueryInterpretation.cppsrc/log_surgeon/SchemaParser.cpp
📚 Learning: 2025-08-08T10:00:20.963Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/VariableQueryToken.hpp:28-31
Timestamp: 2025-08-08T10:00:20.963Z
Learning: In src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp (C++), do not default the comparison operators: the class stores std::vector<std::variant<StaticQueryToken, VariableQueryToken>>, which yields only a weak ordering. A custom operator<=> that maps the variant’s weak ordering to std::strong_ordering is required; avoid suggesting defaulting there in future reviews.
Applied to files:
src/log_surgeon/wildcard_query_parser/QueryInterpretation.cpp
📚 Learning: 2025-08-08T10:21:56.571Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/QueryInterpretation.cpp:57-77
Timestamp: 2025-08-08T10:21:56.571Z
Learning: In src/log_surgeon/wildcard_query_parser/QueryInterpretation.hpp/.cpp (C++), for QueryInterpretation::append_query_interpretation, prefer a single overload taking QueryInterpretation const&; do not suggest adding an rvalue/move overload in future reviews for this method.
Applied to files:
src/log_surgeon/wildcard_query_parser/QueryInterpretation.cpp
📚 Learning: 2025-08-08T10:17:43.495Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/VariableQueryToken.hpp:28-31
Timestamp: 2025-08-08T10:17:43.495Z
Learning: In src/log_surgeon/wildcard_query_parser/VariableQueryToken.hpp/.cpp (C++), do not suggest defaulting operator<=>. The project prefers a custom out-of-line comparator that explicitly handles the bool member (via explicit cast) to avoid implicit conversions; keep the current manual implementation.
Applied to files:
src/log_surgeon/wildcard_query_parser/QueryInterpretation.cpp
📚 Learning: 2025-08-08T10:22:26.739Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#144
File: src/log_surgeon/wildcard_query_parser/QueryInterpretation.cpp:98-120
Timestamp: 2025-08-08T10:22:26.739Z
Learning: In C++ file src/log_surgeon/wildcard_query_parser/QueryInterpretation.cpp, for QueryInterpretation::serialize(), prefer explicit std::holds_alternative/std::get branching over std::visit for readability; do not suggest refactoring to std::visit in future reviews.
Applied to files:
src/log_surgeon/wildcard_query_parser/QueryInterpretation.cpp
📚 Learning: 2025-08-15T01:04:03.681Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: src/log_surgeon/wildcard_query_parser/ExpressionView.cpp:11-11
Timestamp: 2025-08-15T01:04:03.681Z
Learning: In y-scope/log-surgeon C++ codebase, prefer explicit includes in implementation files (.cpp) for types that are directly used, even when those headers are already transitively included through the corresponding header file. This makes dependencies explicit and improves maintainability.
Applied to files:
src/log_surgeon/wildcard_query_parser/QueryInterpretation.cppsrc/log_surgeon/finite_automata/TagOperation.hpp
📚 Learning: 2024-11-27T21:56:13.425Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#56
File: src/log_surgeon/finite_automata/RegisterHandler.hpp:0-0
Timestamp: 2024-11-27T21:56:13.425Z
Learning: In the `log_surgeon` project, header guards in C++ header files should include `_HPP` at the end to match the filename. For example, in `RegisterHandler.hpp`, the header guard should be `LOG_SURGEON_FINITE_AUTOMATA_REGISTER_HANDLER_HPP`.
Applied to files:
src/log_surgeon/finite_automata/RegisterOperation.hpp
📚 Learning: 2025-08-26T10:06:22.914Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#152
File: src/log_surgeon/wildcard_query_parser/Query.hpp:9-11
Timestamp: 2025-08-26T10:06:22.914Z
Learning: In y-scope/log-surgeon project, it's acceptable to include headers like log_surgeon/Lexer.hpp directly in header files rather than using forward declarations, even when only references are used in the interface. The project prefers the simplicity of direct includes over header coupling optimization through forward declarations.
Applied to files:
src/.clang-format
⏰ 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). (6)
- GitHub Check: lint (ubuntu-24.04)
- GitHub Check: lint (macos-15)
- GitHub Check: build (macos-15, debug)
- GitHub Check: build (ubuntu-22.04, debug)
- GitHub Check: build (macos-15, release)
- GitHub Check: build (ubuntu-24.04, debug)
🔇 Additional comments (12)
src/log_surgeon/wildcard_query_parser/QueryInterpretation.cpp (1)
12-13: Include order LGTM and aligns with updated clang-format categories.External fmt headers precede project headers; usage of fmt::format/join covered by included headers.
src/log_surgeon/finite_automata/Nfa.hpp (1)
15-17: fmt includes moved before project headers—consistent and correct.Header uses fmt in inline methods; placement is appropriate.
src/log_surgeon/finite_automata/NfaState.hpp (1)
16-18: Properly prioritizes external fmt headers before project headers.Matches project formatting rules; no functional impact.
src/log_surgeon/finite_automata/DfaState.hpp (1)
15-17: Include reordering matches new policy; fmt usage satisfied.No API/behavioural change.
src/log_surgeon/finite_automata/Dfa.hpp (1)
19-21: fmt includes consolidated early; required for fmt::format and fmt::join. LGTM.Brings <fmt/format.h> and <fmt/ranges.h> up-front and removes duplication elsewhere. Matches the new include-order rules and the file’s usage.
src/log_surgeon/finite_automata/DfaTransition.hpp (1)
14-16: Include reordering aligns with config; dependencies remain intact.Project headers now follow fmt headers. The file uses fmt::join, so keeping <fmt/ranges.h> is correct.
src/log_surgeon/finite_automata/NfaSpontaneousTransition.hpp (1)
15-16: Include order updated; no behavioural change.TagOperation.hpp moved after fmt headers, consistent with the new priorities.
src/log_surgeon/SchemaParser.cpp (1)
26-27: Formatting-only alias wraps; no semantic change.Line breaks in the using aliases are purely stylistic and consistent with clang-format v21.1.
Also applies to: 36-37
tests/test-schema.cpp (1)
30-31: LGTM: formatting-only alias wrap; no semantic change.
Consistent with the updated clang-format rules.src/log_surgeon/finite_automata/RegisterOperation.hpp (1)
10-11: LGTM: include-order adjustment places external before project headers.
Matches the PR’s include policy; no API/ODR impact expected.src/log_surgeon/finite_automata/TagOperation.hpp (1)
10-11: LGTM: include grouping updated to put fmt before project headers.
Change is stylistic; interfaces unchanged.src/log_surgeon/finite_automata/RegexAST.hpp (1)
22-26: LGTM: project header block moved after fmt headers.
Aligns with repo-wide ordering; no functional differences.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.gitignore (1)
2-3: Root-anchored ignores LGTM; resolves prior nit.Using "/.clang-format" and "/.clang-tidy" correctly scopes to the repo root, allowing per-dir configs like src/.clang-format to be tracked.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
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)
.gitignore(1 hunks)lint-requirements.txt(1 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). (7)
- GitHub Check: lint (macos-15)
- GitHub Check: lint (ubuntu-24.04)
- GitHub Check: build (ubuntu-24.04, release)
- GitHub Check: build (macos-15, debug)
- GitHub Check: build (ubuntu-22.04, debug)
- GitHub Check: build (macos-15, release)
- GitHub Check: build (ubuntu-24.04, debug)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
SharafMohamed
approved these changes
Sep 3, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Previously, include statements were ordered and grouped as follows:
This PR swaps project headers (3) and external project headers (4) so that all non-project headers are sequential.
Additionally, we bump dev-utils to get the updated clang-format v21.1 config so CI is successful without locking the version to v20.1.
Checklist
breaking change.
Validation performed
CI passing.
Summary by CodeRabbit