Skip to content

build(deps): Bump fmtlib version to 11.2.0.#153

Merged
Bill-hbrhbr merged 2 commits intoy-scope:mainfrom
Bill-hbrhbr:bump-fmtlib
Aug 26, 2025
Merged

build(deps): Bump fmtlib version to 11.2.0.#153
Bill-hbrhbr merged 2 commits intoy-scope:mainfrom
Bill-hbrhbr:bump-fmtlib

Conversation

@Bill-hbrhbr
Copy link
Contributor

@Bill-hbrhbr Bill-hbrhbr commented Aug 26, 2025

Description

CLP is bumping fmtlib version to 11.2.0, so we need to do the same for log-surgeon to stay consistent across projects.

Bumps fmtlib version and cleans up fmt headers. In 11.2.0:

  • fmt::format should be brought in by <fmt/format.h>
  • fmt::join should be brought in by <fmt/ranges.h>.
  • <fmt/core.h> is obsolete.

Checklist

  • The PR satisfies the contribution guidelines.
  • This is a breaking change and that has been indicated in the PR title, OR this isn't a
    breaking change.
  • Necessary docs have been updated, OR no docs need to be updated.

Validation performed

  • Unit tests pass and clang-tidy has no complaints.

Summary by CodeRabbit

  • Chores
    • Upgraded the formatting library to a newer version, improving build compatibility and stability.
    • Updated build configuration to require the newer dependency.
  • Documentation
    • Revised Requirements to reflect the new minimum version of the formatting library.
  • Tests
    • Adjusted test includes to align with the updated dependency.

No user-facing behaviour changes.

@Bill-hbrhbr Bill-hbrhbr requested a review from a team as a code owner August 26, 2025 05:10
@coderabbitai
Copy link

coderabbitai bot commented Aug 26, 2025

Walkthrough

Bumps the fmt dependency to 11.2.0 and updates includes across headers, sources, build and taskfiles to use fmt/format.h and fmt/ranges.h as required. README and deps taskfile reflect the new minimum version. No logic, control-flow, or public API changes.

Changes

Cohort / File(s) Summary of changes
Finite automata — fmt includes
src/log_surgeon/finite_automata/Dfa.hpp, src/log_surgeon/finite_automata/DfaState.hpp, src/log_surgeon/finite_automata/DfaTransition.hpp, src/log_surgeon/finite_automata/Nfa.hpp, src/log_surgeon/finite_automata/NfaState.hpp, src/log_surgeon/finite_automata/NfaSpontaneousTransition.hpp, src/log_surgeon/finite_automata/RegexAST.hpp, src/log_surgeon/finite_automata/RegisterOperation.hpp, src/log_surgeon/finite_automata/TagOperation.hpp
Replace #include <fmt/core.h> with appropriate fmt/format.h and add fmt/ranges.h where range formatting (e.g., fmt::join) is used. No behavioral or API changes.
Wildcard parser — fmt includes
src/log_surgeon/wildcard_query_parser/QueryInterpretation.cpp
Replace fmt/core.h with fmt/ranges.h while keeping fmt/format.h. No logic changes.
Tests — fmt includes
tests/test-buffer-parser.cpp, tests/test-schema.cpp
Remove or replace fmt/core.h with fmt/format.h as applicable. No test logic changes.
Build & deps
CMakeLists.txt, taskfiles/deps.yaml
Bump required fmt from 8.0.1 to 11.2.0: update find_package version in CMake and tarball URL & SHA256 in deps.yaml.
Docs
README.md
Update minimum required fmt version from 8.0.1 to 11.2.0 in Requirements section.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • davidlion
  • SharafMohamed

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.


📜 Recent 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 8fadd0f and 9d07b59.

📒 Files selected for processing (6)
  • CMakeLists.txt (1 hunks)
  • src/log_surgeon/finite_automata/DfaState.hpp (1 hunks)
  • src/log_surgeon/finite_automata/RegisterOperation.hpp (1 hunks)
  • src/log_surgeon/finite_automata/TagOperation.hpp (1 hunks)
  • tests/test-buffer-parser.cpp (1 hunks)
  • tests/test-schema.cpp (0 hunks)
💤 Files with no reviewable changes (1)
  • tests/test-schema.cpp
🧰 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/TagOperation.hpp
  • src/log_surgeon/finite_automata/RegisterOperation.hpp
  • tests/test-buffer-parser.cpp
  • src/log_surgeon/finite_automata/DfaState.hpp
🧠 Learnings (10)
📚 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/TagOperation.hpp
  • src/log_surgeon/finite_automata/RegisterOperation.hpp
  • src/log_surgeon/finite_automata/DfaState.hpp
📚 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/TagOperation.hpp
  • src/log_surgeon/finite_automata/RegisterOperation.hpp
  • src/log_surgeon/finite_automata/DfaState.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/TagOperation.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/TagOperation.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/RegisterOperation.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/RegisterOperation.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/RegisterOperation.hpp
📚 Learning: 2025-08-15T12:07:58.626Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: tests/test-expression-view.cpp:7-7
Timestamp: 2025-08-15T12:07:58.626Z
Learning: In tests/test-expression-view.cpp, the `<catch2/catch_message.hpp>` header is required for clang-tidy to pass, even though it may not be directly used in the visible code.

Applied to files:

  • tests/test-buffer-parser.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/finite_automata/DfaState.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/DfaState.hpp
⏰ 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: build (ubuntu-24.04, debug)
  • GitHub Check: build (ubuntu-24.04, release)
  • GitHub Check: lint (ubuntu-24.04)
  • GitHub Check: build (ubuntu-22.04, debug)
  • GitHub Check: lint (macos-15)
  • GitHub Check: build (macos-15, debug)
  • GitHub Check: build (macos-15, release)
🔇 Additional comments (5)
src/log_surgeon/finite_automata/TagOperation.hpp (1)

10-10: Header migration to <fmt/format.h> is correct and sufficient here.

This TU only uses fmt::format, so switching from fmt/core.h to fmt/format.h is appropriate. No need for fmt/ranges.h in this file. Build impact should be nil.

tests/test-buffer-parser.cpp (1)

17-17: Tests: include switch to <fmt/format.h> matches usage.

fmt is used only for fmt::format in serialize_id_symbol_map(); <fmt/format.h> is the right include with fmt 11.x. Looks good.

CMakeLists.txt (1)

49-49: fmt 11.2.0 bump verified – includes consistency confirmed, please ensure CI/dependencies updated

All automated include checks passed:

  • No stale #include <fmt/core.h> remains.
  • Every file using fmt::join also includes <fmt/ranges.h>.
  • Every file calling fmt::format(...) includes <fmt/format.h>.

Please manually verify that your CI images (and any optional‐deps CMake pulls) are updated to use fmt ≥ 11.2.0.

src/log_surgeon/finite_automata/DfaState.hpp (1)

22-22: Good call adding <fmt/ranges.h> next to <fmt/format.h>.

This TU uses fmt::join in serialize(), so explicitly adding fmt/ranges.h is required under fmt 11.x. Keeping <fmt/format.h> alongside avoids brittle reliance on transitive includes—aligns with prior feedback.

src/log_surgeon/finite_automata/RegisterOperation.hpp (1)

10-10: Header switch to <fmt/format.h> is appropriate.

This file only needs fmt::format in serialize(); the migration is precise and minimal.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@Bill-hbrhbr Bill-hbrhbr requested a review from davidlion August 26, 2025 05:12
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 85d4f2c and 8fadd0f.

📒 Files selected for processing (10)
  • README.md (1 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/wildcard_query_parser/QueryInterpretation.cpp (1 hunks)
  • taskfiles/deps.yaml (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.hpp
  • src/log_surgeon/finite_automata/NfaSpontaneousTransition.hpp
  • src/log_surgeon/finite_automata/Dfa.hpp
  • src/log_surgeon/finite_automata/RegexAST.hpp
  • src/log_surgeon/finite_automata/DfaState.hpp
  • src/log_surgeon/finite_automata/Nfa.hpp
  • src/log_surgeon/wildcard_query_parser/QueryInterpretation.cpp
  • src/log_surgeon/finite_automata/DfaTransition.hpp
🧠 Learnings (15)
📚 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.hpp
  • src/log_surgeon/finite_automata/NfaSpontaneousTransition.hpp
  • src/log_surgeon/finite_automata/Dfa.hpp
  • src/log_surgeon/finite_automata/RegexAST.hpp
  • src/log_surgeon/finite_automata/DfaState.hpp
  • src/log_surgeon/finite_automata/Nfa.hpp
  • src/log_surgeon/finite_automata/DfaTransition.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.hpp
  • src/log_surgeon/finite_automata/NfaSpontaneousTransition.hpp
  • src/log_surgeon/finite_automata/Dfa.hpp
  • src/log_surgeon/finite_automata/RegexAST.hpp
  • src/log_surgeon/finite_automata/Nfa.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.hpp
  • src/log_surgeon/finite_automata/NfaSpontaneousTransition.hpp
  • src/log_surgeon/finite_automata/Dfa.hpp
  • src/log_surgeon/finite_automata/RegexAST.hpp
  • src/log_surgeon/finite_automata/DfaState.hpp
  • src/log_surgeon/finite_automata/Nfa.hpp
  • src/log_surgeon/finite_automata/DfaTransition.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.hpp
  • src/log_surgeon/finite_automata/RegexAST.hpp
  • src/log_surgeon/finite_automata/Nfa.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.hpp
  • src/log_surgeon/finite_automata/Dfa.hpp
  • src/log_surgeon/finite_automata/RegexAST.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.hpp
  • src/log_surgeon/finite_automata/Dfa.hpp
📚 Learning: 2025-08-15T12:07:58.626Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#150
File: tests/test-expression-view.cpp:7-7
Timestamp: 2025-08-15T12:07:58.626Z
Learning: In tests/test-expression-view.cpp, the `<catch2/catch_message.hpp>` header is required for clang-tidy to pass, even though it may not be directly used in the visible code.

Applied to files:

  • README.md
📚 Learning: 2024-10-11T16:16:02.866Z
Learnt from: SharafMohamed
PR: y-scope/log-surgeon#38
File: src/log_surgeon/finite_automata/RegexAST.hpp:663-669
Timestamp: 2024-10-11T16:16:02.866Z
Learning: In `RegexASTLiteral::serialize()`, to properly handle Unicode characters beyond the ASCII range, cast `m_character` to `char32_t` and use `U"{}{}"` in `fmt::format`.

Applied to files:

  • src/log_surgeon/finite_automata/RegexAST.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/DfaState.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/Nfa.hpp
  • src/log_surgeon/finite_automata/DfaTransition.hpp
📚 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: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-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.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: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
⏰ 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). (5)
  • GitHub Check: lint (ubuntu-24.04)
  • GitHub Check: lint (macos-15)
  • GitHub Check: build (ubuntu-24.04, debug)
  • GitHub Check: build (macos-15, release)
  • GitHub Check: build (macos-15, debug)
🔇 Additional comments (5)
src/log_surgeon/finite_automata/RegexAST.hpp (1)

22-25: Header switch to fmt/format.h is correct for fmt 11.x; xchar/ranges coverage looks complete.

fmt::format is required (join support comes via fmt/ranges.h and U-literals/char32_t via fmt/xchar.h). This aligns with the dependency bump and keeps Unicode serialisation intact.

src/log_surgeon/wildcard_query_parser/QueryInterpretation.cpp (1)

12-14: Adding fmt/ranges.h is appropriate for fmt::join usage.

The file already includes fmt/format.h; together these ensure the serialisation at Lines 93–97 formats correctly. No behavioural change.

src/log_surgeon/finite_automata/NfaState.hpp (1)

23-25: Replacing fmt/core.h with fmt/ranges.h matches the code’s fmt::join usage.

fmt/format.h remains for fmt::format, so this is the right combination for fmt 11.2.0.

src/log_surgeon/finite_automata/NfaSpontaneousTransition.hpp (1)

14-16: Including fmt/ranges.h is necessary here due to views + fmt::join.

This matches the transform(view) pattern in serialize() and the project-wide update.

src/log_surgeon/finite_automata/DfaTransition.hpp (1)

15-15: Adding fmt/ranges.h is correct for join-based serialisation.

This aligns with fmt ≥ 11 behaviour and local usage.

Copy link
Member

@davidlion davidlion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants