build(deps): Bump fmtlib to v11.2.0, spdlog to v1.15.3, and log-surgeon to y-scope/log-surgeon@a722d07. #1263
Conversation
WalkthroughStandardizes fmt header includes across sources/tests (adding Changes
Sequence Diagram(s)(omitted — changes are header/include and dependency updates only; no control-flow or runtime behavior modifications) Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Pre-merge checks and finishing touches❌ Failed checks
✅ Passed checks (1 passed)
✨ 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp (1)
393-405: Optional: prefer fmt::to_string over format("{}") for join resultsWhen converting fmt::join(...) to std::string, fmt::to_string(join_view) is a touch clearer and avoids a redundant format string. This is a test-only path, so treat as a readability-only tweak.
Example change pattern:
- auto const matchable_kql_query_str{ - fmt::format("{}", fmt::join(matchable_kql_expressions, " OR ")) - }; + auto const matchable_kql_query_str = fmt::to_string(fmt::join(matchable_kql_expressions, " OR "));
📜 Review details
Configuration used: CodeRabbit UI
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 (3)
components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp(1 hunks)components/core/src/clp/ffi/ir_stream/search/test/utils.cpp(1 hunks)taskfiles/deps/main.yaml(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit configuration file
- Prefer
false == <expression>rather than!<expression>.
Files:
components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cppcomponents/core/src/clp/ffi/ir_stream/search/test/utils.cpp
🧠 Learnings (2)
📓 Common learnings
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1122
File: components/core/src/clp/clp/CMakeLists.txt:175-195
Timestamp: 2025-07-23T09:54:45.185Z
Learning: In the CLP project, when reviewing CMakeLists.txt changes that introduce new compression library dependencies (BZip2, LibLZMA, LZ4, ZLIB), the team prefers to address conditional linking improvements in separate PRs rather than expanding the scope of focused migration PRs like the LibArchive task-based installation migration.
📚 Learning: 2024-11-01T03:26:26.386Z
Learnt from: LinZhihao-723
PR: y-scope/clp#570
File: components/core/tests/test-ir_encoding_methods.cpp:376-399
Timestamp: 2024-11-01T03:26:26.386Z
Learning: In the test code (`components/core/tests/test-ir_encoding_methods.cpp`), exception handling for `msgpack::unpack` can be omitted because the Catch2 testing framework captures exceptions if they occur.
Applied to files:
components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp
🔇 Additional comments (9)
components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp (3)
14-14: Adding fmt/ranges is correct and required for fmt::join on sets/vectorsIncluding <fmt/ranges.h> is necessary for the fmt::join calls over std::set/std::vector throughout this file and prevents ADL/formatter misses with newer fmt. Looks good.
Also applies to: 236-237, 394-395, 397-398, 401-402, 452-453
210-216: Guideline check: boolean style matches repo preferenceUse of comparisons like
false == matchable_node_ids.empty()aligns with the stated coding guideline to preferfalse == <expr>over!<expr>. No action needed.
231-239: Stable serialisation of node-id mapsUsing fmt::join over std::set ensures deterministic ordering in the captured diagnostics, which helps when diffing test failures. Good choice.
components/core/src/clp/ffi/ir_stream/search/test/utils.cpp (2)
15-15: Include fmt/ranges to enable join over containersThis header is required for
fmt::join(m_matchable_node_ids, ",")in serialize(). Correct and future-proof with fmt 11.x.Also applies to: 60-66
26-45: Early return on non-matchable types is concise and avoids workThe bitmask gate
(type & m_matchable_types) == 0is a clean fast-path. LGTM.taskfiles/deps/main.yaml (4)
253-255: fmt bump to 11.2.0: header usage is consistent with testsTests now include fmt/ranges.h explicitly, which is the right pairing with fmt 11.x. No follow-on CMake changes appear necessary.
431-443: Keep SPDLOG_FMT_EXTERNAL to avoid ODR/ABI collisionsGood to see
-DSPDLOG_FMT_EXTERNAL=ONretained. With fmt updated across the tree, this prevents spdlog from vendoring a mismatched fmt and protects against ODR issues.
314-321: fmt-dependent consumer task already depends on fmtlog-surgeon depends on fmt and uses generated CMake settings; this should pick up the new fmt without manual nits. No changes needed.
444-449: Manual verification required: SHA256 checksums and fmt compatibility for spdlog v1.15.3The automated checksum script couldn’t run in this environment (missing
sha256sum/openssl), and the fmt-compatibility web query was inconclusive. Please:
Locally verify the SHA256 of both tarballs using your preferred tooling:
- fmt 11.2.0:
https://github.com/fmtlib/fmt/archive/refs/tags/11.2.0.tar.gz→bc23066d87ab3168f27cef3e97d545fa63314f5c79df5ea444d41d56f962c6af- spdlog 1.15.3:
https://github.com/gabime/spdlog/archive/refs/tags/v1.15.3.tar.gz→15a04e69c222eb6c01094b5c7ff8a249b36bb22788d72519646fb85feb267e67Confirm that spdlog v1.15.3 officially supports fmt 11.x (including 11.2.0) when built with
-DSPDLOG_FMT_EXTERNAL=ONby checking the v1.15.3 release notes or CMake/README documentation.No code changes proposed here—just a manual verification step.
components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
taskfiles/deps/main.yaml (2)
427-449: spdlog: version bump and external fmt flags look consistent; add VERSION var and verify checksum/URL
- SPDLOG_FMT_EXTERNAL=ON remains set, which is required when linking against external fmt 11.x.
- Consider extracting VERSION similar to fmt for maintainability.
- Please verify the SHA256 against upstream.
Refactor:
spdlog: internal: true run: "once" deps: - "fmt" cmds: - task: "utils:install-remote-cmake-lib" vars: + VERSION: "1.15.3" CMAKE_GEN_ARGS: - "-C {{.G_DEPS_CORE_CMAKE_SETTINGS_DIR}}/{{.G_FMT_LIB_NAME}}.cmake" - "-DCMAKE_BUILD_TYPE=Release" - "-DCMAKE_INSTALL_MESSAGE=LAZY" - "-DCMAKE_POLICY_DEFAULT_CMP0074=NEW" - "-DSPDLOG_BUILD_EXAMPLE=OFF" - "-DSPDLOG_BUILD_EXAMPLE_HO=OFF" - "-DSPDLOG_FMT_EXTERNAL=ON" LIB_NAME: "spdlog" - TARBALL_SHA256: "15a04e69c222eb6c01094b5c7ff8a249b36bb22788d72519646fb85feb267e67" + TARBALL_SHA256: "15a04e69c222eb6c01094b5c7ff8a249b36bb22788d72519646fb85feb267e67" # NOTE: Since spdlog depends on fmt, we need to choose a version of spdlog that's # compatible with the version of fmt we use. - TARBALL_URL: "https://github.com/gabime/spdlog/archive/refs/tags/v1.15.3.tar.gz" + TARBALL_URL: "https://github.com/gabime/spdlog/archive/refs/tags/v{{.VERSION}}.tar.gz"
241-255: Extract a reusableVERSIONvariable for fmtfmt was bumped to 11.2.0 correctly, and the SHA256 checksum has been verified against the upstream tarball. To minimise future drift, extract the version into a
VERSIONvariable and reuse it in the download URL.Affected location:
- taskfiles/deps/main.yaml, lines 241–255
Proposed refactor:
fmt: internal: true run: "once" cmds: - task: "utils:install-remote-cmake-lib" vars: + VERSION: "11.2.0" CMAKE_GEN_ARGS: - "-DCMAKE_BUILD_TYPE=Release" - "-DCMAKE_INSTALL_MESSAGE=LAZY" - "-DFMT_DOC=OFF" - "-DFMT_TEST=OFF" LIB_NAME: "{{.G_FMT_LIB_NAME}}" TARBALL_SHA256: "bc23066d87ab3168f27cef3e97d545fa63314f5c79df5ea444d41d56f962c6af" - TARBALL_URL: "https://github.com/fmtlib/fmt/archive/refs/tags/11.2.0.tar.gz" + TARBALL_URL: "https://github.com/fmtlib/fmt/archive/refs/tags/{{.VERSION}}.tar.gz"(optional enhancement) By centralizing the version, updates will only require a single change and reduce the chance of URL/checksum mismatches down the line.
📜 Review details
Configuration used: CodeRabbit UI
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)
docs/src/dev-guide/components-core/index.md(2 hunks)taskfiles/deps/main.yaml(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-25T07:27:19.449Z
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1262
File: docs/src/dev-guide/components-core/index.md:50-50
Timestamp: 2025-08-25T07:27:19.449Z
Learning: In the CLP project, nlohmann_json version 3.11.3 is installed through their dependency management process, but they don't enforce version constraints in CMake find_package calls - they control versions through installation rather than CMake version pinning.
Applied to files:
docs/src/dev-guide/components-core/index.md
⏰ 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). (8)
- GitHub Check: centos-stream-9-dynamic-linked-bins
- GitHub Check: ubuntu-jammy-static-linked-bins
- GitHub Check: centos-stream-9-static-linked-bins
- GitHub Check: ubuntu-jammy-dynamic-linked-bins
- GitHub Check: ubuntu-jammy-lint
- GitHub Check: build-macos (macos-15, true)
- GitHub Check: build-macos (macos-15, false)
- GitHub Check: lint-check (macos-15)
🔇 Additional comments (3)
docs/src/dev-guide/components-core/index.md (1)
43-43: Docs: fmt version bump reflected correctlyfmt is updated to v11.2.0, matching the Taskfile change. No further issues spotted in this row.
taskfiles/deps/main.yaml (2)
304-322: Downstream consumer: log-surgeon pinned commit still builds with fmt 11/spdlog 1.15?log-surgeon depends on fmt and GSL via CMake settings files. Given the fmt major bump, ensure that log-surgeon’s minimum required fmt aligns with 11.x (header paths and APIs unchanged for our usage). CI passed per PR description, but if there are any optional components or examples disabled locally, we should confirm they’re not masking incompatibilities.
67-90: Please confirm intended execution mode forcore-all-parallelIt looks like the
core-all-paralleltask now usescmds:(which runs each sub-task sequentially) rather thandeps:(which runs them in parallel). Since the name still includes “parallel”, this may either be an accidental performance regression or an intentional change for determinism.If sequential execution is desired, please either:
- Rename the task (e.g. to
core-all), or- Add a brief comment explaining why you’ve chosen sequential installs over parallel.
If you still want parallel installs, switch back to
deps::core-all-parallel: internal: true run: "once" - cmds: + deps: - task: "absl" - task: "antlr-jar" - task: "antlr-runtime" - task: "boost" - task: "catch2" - task: "date" - task: "fmt" - task: "liblzma" - task: "log-surgeon" - task: "lz4" - task: "microsoft.gsl" - task: "mongocxx" - task: "msgpack-cxx" - task: "nlohmann_json" - task: "simdjson" - task: "spdlog" - task: "sqlite3" - task: "utfcpp" - task: "yaml-cpp" - task: "ystdlib" - task: "zlib" - task: "zstd"
Bill-hbrhbr
changed the title
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
components/core/src/clp/CurlDownloadHandler.cpp (2)
50-50: Prefer explicit type for libcurl boolean optionUse an explicit long for consistency with other options.
- m_easy_handle.set_option(CURLOPT_NOPROGRESS, 0); + m_easy_handle.set_option(CURLOPT_NOPROGRESS, static_cast<long>(false));
115-116: Typo in comment: “reponse” → “response”Minor spelling fix.
- // Set up failure on HTTP error reponse + // Set up failure on HTTP error responsecomponents/core/src/glt/GlobalMetadataDBConfig.cpp (2)
25-26: Adopt project boolean style (“false == expr”)Align with repo guideline preferring “false == …” over negation.
- if (!config["type"]) { + if (false == config["type"]) { - if (!config["host"]) { + if (false == config["host"]) { - if (!config["port"]) { + if (false == config["port"]) { - if (!config["name"]) { + if (false == config["name"]) { - if (!config["username"]) { + if (false == config["username"]) { - if (!config["password"]) { + if (false == config["password"]) { - if (!config["table_prefix"]) { + if (false == config["table_prefix"]) {Also applies to: 35-37, 47-49, 59-61, 71-73, 83-85, 95-97
10-19: Helper functions should not throw and pretend to returnThese functions throw internally yet have a return type, which is misleading. Return the exception object and let callers throw it.
-static exception get_yaml_missing_key_exception(string const& key_name) { - throw invalid_argument(fmt::format("Missing key '{}'", key_name)); -} +static invalid_argument get_yaml_missing_key_exception(string const& key_name) { + return invalid_argument(fmt::format("Missing key '{}'", key_name)); +} -static exception -get_yaml_unconvertable_value_exception(string const& key_name, string const& destination_type) { - throw invalid_argument( - fmt::format("'{}' could not be converted to type '{}'", key_name, destination_type) - ); -} +static invalid_argument +get_yaml_unconvertable_value_exception(string const& key_name, string const& destination_type) { + return invalid_argument( + fmt::format("'{}' could not be converted to type '{}'", key_name, destination_type) + ); +}Call sites like
throw get_yaml_missing_key_exception("type");remain valid.components/core/src/clp/database_utils.cpp (1)
15-26: Add precondition guards to avoid OOB/UB on empty inputsThese helpers index element 0 or assume begin/end ranges > 0; zero-sized vectors or num_placeholders = 0 will read OOB. Either document preconditions or guard with fast-fail/asserts.
Example pattern:
// At function start(s) if (field_names_and_types.empty()) { return {}; } // or assert(!empty) if (field_names.empty()) { return {}; } if (0 == num_placeholders) { return {}; } if (begin_ix >= end_ix || end_ix > field_names.size()) { return {}; } // likewise for pairsAlso applies to: 35-40, 49-54, 63-68, 77-82, 91-96, 108-113, 122-127
components/core/src/glt/database_utils.cpp (1)
15-26: Defensive checks recommended to prevent OOB on empty inputsSame index-at-0 and range assumptions as the clp variant; add guards or asserts to encode preconditions.
Minimal pattern is identical to the one suggested in clp/database_utils.cpp.
Also applies to: 35-40, 49-54, 63-68, 77-82, 91-96, 108-113, 122-127
📜 Review details
Configuration used: CodeRabbit UI
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 (20)
components/core/src/clp/CurlDownloadHandler.cpp(1 hunks)components/core/src/clp/GlobalMetadataDBConfig.cpp(1 hunks)components/core/src/clp/GlobalMySQLMetadataDB.cpp(1 hunks)components/core/src/clp/GlobalSQLiteMetadataDB.cpp(1 hunks)components/core/src/clp/database_utils.cpp(1 hunks)components/core/src/clp/ffi/ir_stream/search/test/test_deserializer_integration.cpp(1 hunks)components/core/src/clp/streaming_archive/ArchiveMetadata.cpp(0 hunks)components/core/src/clp/streaming_archive/MetadataDB.cpp(1 hunks)components/core/src/clp_s/CommandLineArguments.cpp(1 hunks)components/core/src/clp_s/JsonConstructor.cpp(1 hunks)components/core/src/clp_s/Utils.cpp(1 hunks)components/core/src/clp_s/indexer/MySQLIndexStorage.cpp(1 hunks)components/core/src/clp_s/kv_ir_search.cpp(1 hunks)components/core/src/glt/GlobalMetadataDBConfig.cpp(1 hunks)components/core/src/glt/GlobalMySQLMetadataDB.cpp(1 hunks)components/core/src/glt/GlobalSQLiteMetadataDB.cpp(1 hunks)components/core/src/glt/database_utils.cpp(1 hunks)components/core/src/glt/streaming_archive/MetadataDB.cpp(1 hunks)components/core/tests/test-NetworkReader.cpp(1 hunks)components/core/tests/test-SQLiteDB.cpp(1 hunks)
💤 Files with no reviewable changes (1)
- components/core/src/clp/streaming_archive/ArchiveMetadata.cpp
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit configuration file
- Prefer
false == <expression>rather than!<expression>.
Files:
components/core/src/clp_s/CommandLineArguments.cppcomponents/core/src/clp/GlobalMySQLMetadataDB.cppcomponents/core/src/clp/streaming_archive/MetadataDB.cppcomponents/core/src/clp_s/indexer/MySQLIndexStorage.cppcomponents/core/src/clp/ffi/ir_stream/search/test/test_deserializer_integration.cppcomponents/core/tests/test-NetworkReader.cppcomponents/core/src/clp/GlobalSQLiteMetadataDB.cppcomponents/core/src/clp_s/JsonConstructor.cppcomponents/core/src/glt/GlobalMySQLMetadataDB.cppcomponents/core/tests/test-SQLiteDB.cppcomponents/core/src/glt/GlobalMetadataDBConfig.cppcomponents/core/src/clp/GlobalMetadataDBConfig.cppcomponents/core/src/clp/database_utils.cppcomponents/core/src/glt/database_utils.cppcomponents/core/src/glt/GlobalSQLiteMetadataDB.cppcomponents/core/src/clp_s/Utils.cppcomponents/core/src/glt/streaming_archive/MetadataDB.cppcomponents/core/src/clp_s/kv_ir_search.cppcomponents/core/src/clp/CurlDownloadHandler.cpp
🧠 Learnings (7)
📓 Common learnings
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1122
File: components/core/src/clp/clp/CMakeLists.txt:175-195
Timestamp: 2025-07-23T09:54:45.185Z
Learning: In the CLP project, when reviewing CMakeLists.txt changes that introduce new compression library dependencies (BZip2, LibLZMA, LZ4, ZLIB), the team prefers to address conditional linking improvements in separate PRs rather than expanding the scope of focused migration PRs like the LibArchive task-based installation migration.
📚 Learning: 2024-10-13T09:27:43.408Z
Learnt from: LinZhihao-723
PR: y-scope/clp#557
File: components/core/tests/test-ir_encoding_methods.cpp:1216-1286
Timestamp: 2024-10-13T09:27:43.408Z
Learning: In the unit test case `ffi_ir_stream_serialize_schema_tree_node_id` in `test-ir_encoding_methods.cpp`, suppressing the `readability-function-cognitive-complexity` warning is acceptable due to the expansion of Catch2 macros in C++ tests, and such test cases may not have readability issues.
Applied to files:
components/core/src/clp/ffi/ir_stream/search/test/test_deserializer_integration.cpp
📚 Learning: 2024-10-14T03:42:10.355Z
Learnt from: LinZhihao-723
PR: y-scope/clp#558
File: components/core/tests/test-ffi_KeyValuePairLogEvent.cpp:14-14
Timestamp: 2024-10-14T03:42:10.355Z
Learning: In the file `components/core/tests/test-ffi_KeyValuePairLogEvent.cpp`, including `<json/single_include/nlohmann/json.hpp>` is consistent with the project's coding standards.
Applied to files:
components/core/src/clp/ffi/ir_stream/search/test/test_deserializer_integration.cppcomponents/core/tests/test-NetworkReader.cppcomponents/core/src/clp_s/JsonConstructor.cppcomponents/core/src/clp_s/kv_ir_search.cpp
📚 Learning: 2024-11-15T03:15:45.919Z
Learnt from: LinZhihao-723
PR: y-scope/clp#593
File: components/core/tests/test-NetworkReader.cpp:216-219
Timestamp: 2024-11-15T03:15:45.919Z
Learning: In the `network_reader_with_valid_http_header_kv_pairs` test case in `components/core/tests/test-NetworkReader.cpp`, additional error handling for JSON parsing failures is not necessary, as the current error message is considered sufficient.
Applied to files:
components/core/tests/test-NetworkReader.cpp
📚 Learning: 2024-10-07T21:16:41.660Z
Learnt from: AVMatthews
PR: y-scope/clp#543
File: components/core/src/clp_s/JsonParser.cpp:769-779
Timestamp: 2024-10-07T21:16:41.660Z
Learning: In `components/core/src/clp_s/JsonParser.cpp`, when handling errors in `parse_from_ir`, prefer to maintain the current mix of try-catch and if-statements because specific messages are returned back up in some cases.
Applied to files:
components/core/src/clp_s/JsonConstructor.cpp
📚 Learning: 2025-07-01T14:49:07.333Z
Learnt from: jackluo923
PR: y-scope/clp#1054
File: components/core/src/glt/SQLitePreparedStatement.hpp:4-4
Timestamp: 2025-07-01T14:49:07.333Z
Learning: In the CLP codebase, standard headers like `<cstdint>` and `<string>` in alphabetical order (as seen in components/core/src/glt/SQLitePreparedStatement.hpp) are correctly ordered and do not need adjustment.
Applied to files:
components/core/tests/test-SQLiteDB.cppcomponents/core/src/clp/database_utils.cpp
📚 Learning: 2025-01-30T19:26:33.869Z
Learnt from: davemarco
PR: y-scope/clp#700
File: components/core/src/clp/streaming_archive/ArchiveMetadata.hpp:153-155
Timestamp: 2025-01-30T19:26:33.869Z
Learning: When working with constexpr strings (string literals with static storage duration), std::string_view is the preferred choice for member variables as it's more efficient and safe, avoiding unnecessary memory allocations.
Applied to files:
components/core/src/clp_s/kv_ir_search.cpp
🔇 Additional comments (12)
components/core/tests/test-NetworkReader.cpp (1)
15-15: Correct fmt include for fmt::formatSwitching to
<fmt/format.h>is the right header forfmt::formatwith fmt ≥11. LGTM.components/core/src/clp/GlobalMetadataDBConfig.cpp (1)
11-11: Aligns with fmt 11.x header layoutUsing
<fmt/format.h>matches the file’s use offmt::format. No further changes needed here.components/core/src/clp_s/kv_ir_search.cpp (1)
11-11: Swapping to fmt/format.h is correctMatches the file’s use of
fmt::format. No further changes needed.components/core/src/clp_s/CommandLineArguments.cpp (1)
8-8: fmt include update is appropriate
fmt::formatis used throughout;fmt/format.his the right header.components/core/src/clp_s/JsonConstructor.cpp (1)
7-7: fmt include migration looks correctSwitching to <fmt/format.h> matches the file’s use of fmt::format; no further changes needed.
components/core/src/clp/CurlDownloadHandler.cpp (1)
17-17: Header update aligns with fmt 11<fmt/format.h> is appropriate for fmt::format usage here.
components/core/src/glt/GlobalMetadataDBConfig.cpp (1)
3-3: fmt include migration looks correctUsing <fmt/format.h> is sufficient for fmt::format calls.
components/core/src/clp_s/Utils.cpp (1)
10-10: fmt include update is appropriate<fmt/format.h> matches the file’s usage of fmt::format; no other changes needed.
components/core/src/clp/database_utils.cpp (1)
3-4: LGTM: fmt header migration is correct for fmt 11.xIncluding <fmt/base.h> (memory_buffer) and <fmt/format.h> (format/format_to) matches the used APIs. No behavioural change.
components/core/src/clp/ffi/ir_stream/search/test/test_deserializer_integration.cpp (1)
13-13: LGTM: use <fmt/format.h> in testsfmt::format is provided by <fmt/format.h>; no need for <fmt/base.h> here. Change is correct.
components/core/src/glt/database_utils.cpp (1)
3-4: LGTM: fmt header split aligns with 11.x<fmt/base.h> + <fmt/format.h> matches memory_buffer/format_to usage. Safe, no functional changes.
components/core/src/glt/streaming_archive/MetadataDB.cpp (1)
5-6: LGTM: fmt header updates match usageBoth fmt::memory_buffer and fmt::format_to are used; including <fmt/base.h> and <fmt/format.h> is correct. spdlog formatting calls remain compatible with fmt 11.x.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
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)
docs/src/dev-docs/components-core/index.md(1 hunks)taskfiles/deps/main.yaml(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1122
File: components/core/src/clp/clp/CMakeLists.txt:175-195
Timestamp: 2025-07-23T09:54:45.185Z
Learning: In the CLP project, when reviewing CMakeLists.txt changes that introduce new compression library dependencies (BZip2, LibLZMA, LZ4, ZLIB), the team prefers to address conditional linking improvements in separate PRs rather than expanding the scope of focused migration PRs like the LibArchive task-based installation migration.
📚 Learning: 2025-08-25T07:27:19.449Z
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1262
File: docs/src/dev-guide/components-core/index.md:50-50
Timestamp: 2025-08-25T07:27:19.449Z
Learning: In the CLP project, nlohmann_json version 3.11.3 is installed through their dependency management process, but they don't enforce version constraints in CMake find_package calls - they control versions through installation rather than CMake version pinning.
Applied to files:
docs/src/dev-docs/components-core/index.md
⏰ 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: centos-stream-9-dynamic-linked-bins
- GitHub Check: ubuntu-jammy-dynamic-linked-bins
- GitHub Check: ubuntu-jammy-static-linked-bins
- GitHub Check: ubuntu-jammy-lint
- GitHub Check: centos-stream-9-static-linked-bins
- GitHub Check: build-macos (macos-15, false)
- GitHub Check: build-macos (macos-15, true)
🔇 Additional comments (1)
taskfiles/deps/main.yaml (1)
444-449: Ignore the spdlog 1.15.3 upgrade suggestion without a matching fmt bumpUpon inspection, the taskfile is pinning fmt at v10.1.1 (see lines 254–258 of
taskfiles/deps/main.yaml), and spdlog v1.12.0 was chosen to match that fmt version underSPDLOG_FMT_EXTERNAL=ON. Upgrading spdlog alone to v1.15.3—while still using fmt 10.1.1—will lead to build-time incompatibilities.• The fmt block is at v10.1.1:
TARBALL_SHA256: "78b8c0a72b1c35e4443a7e308df52498252d1cefc2b08c9a97bc9ee6cfe61f8b" TARBALL_URL: "https://github.com/fmtlib/fmt/archive/refs/tags/10.1.1.tar.gz"• The spdlog block (lines 436–449) uses v1.12.0 to align with fmt 10.1.1.
• If you do want spdlog v1.15.3, you must also upgrade fmt to an 11.x release—updating both itsTARBALL_URLandTARBALL_SHA256. Otherwise, keep spdlog at v1.12.0 to maintain compatibility.No changes are required here as long as fmt remains at v10.1.1.
Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/src/dev-docs/components-core/index.md (1)
36-56: Synchronize Taskfile pins with documented versions for reproducible buildsThe component-core docs (
docs/src/dev-docs/components-core/index.md) list these versions:
- fmt v11.2.0
- spdlog v1.15.3
- log-surgeon 831a9f9
But
taskfiles/deps/main.yamlstill pins:
- fmt 10.1.1 (
…/fmt/archive/refs/tags/10.1.1.tar.gz)- spdlog v1.12.0
- log-surgeon f92f501 (from the Bill-hbrhbr fork)
This mismatch will confuse users and break reproducibility. Please update the Taskfile to match the docs (or vice versa if downgrading is intended). Given this PR’s goal to adopt newer versions, I recommend bumping the Taskfile:
• In taskfiles/deps/main.yaml (≈ line 252), update fmt’s
TARBALL_URLto
https://github.com/fmtlib/fmt/archive/refs/tags/11.2.0.tar.gz
and refresh the correspondingTARBALL_SHA256.
• In taskfiles/deps/main.yaml (≈ line 443), update spdlog’sTARBALL_URLto
https://github.com/gabime/spdlog/archive/refs/tags/v1.15.3.tar.gz
and refresh itsTARBALL_SHA256.
• In taskfiles/deps/main.yaml (≈ line 319), point log-surgeon’sTARBALL_URLto the official y-scope repo at
https://github.com/y-scope/log-surgeon/archive/831a9f9.tar.gz
and refresh itsTARBALL_SHA256.Ensuring docs and build scripts use the same versions is critical for a reproducible and user-friendly setup.
taskfiles/deps/main.yaml (1)
241-255: fmt dependency bump: update tarball URL and checksum to v11.2.0The pin at v10.1.1 conflicts with the PR’s target of v11.2.0 and can cause header/API mismatches. Please update the
TARBALL_URLandTARBALL_SHA256intaskfiles/deps/main.yaml(lines 241–255) as follows:- TARBALL_SHA256: "78b8c0a72b1c35e4443a7e308df52498252d1cefc2b08c9a97bc9ee6cfe61f8b" - TARBALL_URL: "https://github.com/fmtlib/fmt/archive/refs/tags/10.1.1.tar.gz" + TARBALL_SHA256: "bc23066d87ab3168f27cef3e97d545fa63314f5c79df5ea444d41d56f962c6af" + TARBALL_URL: "https://github.com/fmtlib/fmt/archive/refs/tags/11.2.0.tar.gz"This ensures the downloaded archive matches fmt v11.2.0’s contents and APIs.
📜 Review details
Configuration used: CodeRabbit UI
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)
docs/src/dev-docs/components-core/index.md(1 hunks)taskfiles/deps/main.yaml(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1122
File: components/core/src/clp/clp/CMakeLists.txt:175-195
Timestamp: 2025-07-23T09:54:45.185Z
Learning: In the CLP project, when reviewing CMakeLists.txt changes that introduce new compression library dependencies (BZip2, LibLZMA, LZ4, ZLIB), the team prefers to address conditional linking improvements in separate PRs rather than expanding the scope of focused migration PRs like the LibArchive task-based installation migration.
📚 Learning: 2025-08-25T07:27:19.449Z
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1262
File: docs/src/dev-guide/components-core/index.md:50-50
Timestamp: 2025-08-25T07:27:19.449Z
Learning: In the CLP project, nlohmann_json version 3.11.3 is installed through their dependency management process, but they don't enforce version constraints in CMake find_package calls - they control versions through installation rather than CMake version pinning.
Applied to files:
docs/src/dev-docs/components-core/index.md
⏰ 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). (1)
- GitHub Check: ubuntu-jammy-lint
🔇 Additional comments (1)
taskfiles/deps/main.yaml (1)
64-90: Add guardrails for version sync & remove deprecated fmt/core.h include
- A deprecated include of
<fmt/core.h>remains incomponents/core/tests/test-kql.cpp(line 7). Please update to the current API (e.g.<fmt/fmt.h>) or otherwise handle this include.- The automated extraction of versions from
docs/src/dev-docs/components-core/index.mddid not yield values. Manually verify that the versions listed in the docs for fmt, spdlog, and log-surgeon exactly match the pinned versions intaskfiles/deps/main.yaml.- Once verified, consider adding a CI or pre-commit guard to:
- Assert no occurrences of
fmt/core.h.- Compare docs’ version entries against Taskfile pins for fmt, spdlog, and log-surgeon.
This reverts commit dc54e45.
|
review again all the header includes with fmtlib 11.2.0 |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 6544 🏁 Script executed: Length of output: 5115 @Bill-hbrhbr I've reviewed all the fmt header includes for fmtlib 11.2.0 compatibility. Here's what I found: ✅ Good NewsMost files have been correctly updated for fmt 11.2.0:
❌ One Remaining IssueThere's one file that still needs updating:
✅ Verification SummaryAll other files (40+ files) have been correctly migrated from the obsolete Please update the remaining file and the fmt header migration will be complete! 🧠 Learnings used |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
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 (1)
taskfiles/deps/main.yaml(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1122
File: components/core/src/clp/clp/CMakeLists.txt:175-195
Timestamp: 2025-07-23T09:54:45.185Z
Learning: In the CLP project, when reviewing CMakeLists.txt changes that introduce new compression library dependencies (BZip2, LibLZMA, LZ4, ZLIB), the team prefers to address conditional linking improvements in separate PRs rather than expanding the scope of focused migration PRs like the LibArchive task-based installation migration.
⏰ 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). (12)
- GitHub Check: ubuntu-jammy-dynamic-linked-bins
- GitHub Check: ubuntu-jammy-lint
- GitHub Check: ubuntu-jammy-static-linked-bins
- GitHub Check: centos-stream-9-static-linked-bins
- GitHub Check: centos-stream-9-dynamic-linked-bins
- GitHub Check: musllinux_1_2-x86_64-static-linked-bins
- GitHub Check: musllinux_1_2-x86_64-dynamic-linked-bins
- GitHub Check: manylinux_2_28-x86_64-dynamic-linked-bins
- GitHub Check: manylinux_2_28-x86_64-static-linked-bins
- GitHub Check: build-macos (macos-15, true)
- GitHub Check: build-macos (macos-15, false)
- GitHub Check: lint-check (macos-15)
🔇 Additional comments (2)
taskfiles/deps/main.yaml (2)
444-448: spdlog v1.15.3 with external fmt — LGTM.Flags include SPDLOG_FMT_EXTERNAL=ON and inherit fmt settings; compatible with fmt 11.
Please confirm a clean rebuild of dependants that include spdlog headers only (if any), since header-only consumers sometimes require matching compile defs.
253-254: LGTM — fmt bumped to 11.2.0 with verified checksum
Checksum aligns with the upstream release.
|
create an issue in this repo to bump Velox fmtlib version from 10.1.x to 11.2. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainI'll create an issue to track bumping the Velox fmtlib version to match the main project's updated dependency. @Bill-hbrhbr I've created an issue to track bumping the Velox fmtlib version from 10.1.x to 11.2.0. The issue includes:
This will help ensure the Velox subproject stays aligned with the main project's fmtlib version. 🏁 Script executed: Length of output: 1190 @Bill-hbrhbr Perfect! I've successfully created issue #1272 to track bumping the Velox fmtlib version from 10.1.x to 11.2.0. The issue is now available at: #1272 It includes all the necessary context, references to this PR and your comment, and has been assigned to you for tracking. This will help ensure the Velox subproject stays aligned with the main CLP project's updated fmtlib dependency. 🐰✨ 🧠 Learnings used |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
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 (1)
components/core/tests/test-kql.cpp(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit configuration file
- Prefer
false == <expression>rather than!<expression>.
Files:
components/core/tests/test-kql.cpp
🧠 Learnings (2)
📓 Common learnings
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1122
File: components/core/src/clp/clp/CMakeLists.txt:175-195
Timestamp: 2025-07-23T09:54:45.185Z
Learning: In the CLP project, when reviewing CMakeLists.txt changes that introduce new compression library dependencies (BZip2, LibLZMA, LZ4, ZLIB), the team prefers to address conditional linking improvements in separate PRs rather than expanding the scope of focused migration PRs like the LibArchive task-based installation migration.
📚 Learning: 2024-10-14T03:42:10.355Z
Learnt from: LinZhihao-723
PR: y-scope/clp#558
File: components/core/tests/test-ffi_KeyValuePairLogEvent.cpp:14-14
Timestamp: 2024-10-14T03:42:10.355Z
Learning: In the file `components/core/tests/test-ffi_KeyValuePairLogEvent.cpp`, including `<json/single_include/nlohmann/json.hpp>` is consistent with the project's coding standards.
Applied to files:
components/core/tests/test-kql.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). (11)
- GitHub Check: build-macos (macos-15, true)
- GitHub Check: build-macos (macos-15, false)
- GitHub Check: ubuntu-jammy-lint
- GitHub Check: ubuntu-jammy-dynamic-linked-bins
- GitHub Check: ubuntu-jammy-static-linked-bins
- GitHub Check: centos-stream-9-dynamic-linked-bins
- GitHub Check: musllinux_1_2-x86_64-static-linked-bins
- GitHub Check: centos-stream-9-static-linked-bins
- GitHub Check: musllinux_1_2-x86_64-dynamic-linked-bins
- GitHub Check: manylinux_2_28-x86_64-dynamic-linked-bins
- GitHub Check: manylinux_2_28-x86_64-static-linked-bins
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp(1 hunks)components/core/src/clp/ffi/ir_stream/search/test/utils.cpp(1 hunks)components/core/src/clp_s/CommandLineArguments.cpp(1 hunks)components/core/src/clp_s/Utils.cpp(1 hunks)components/core/src/clp_s/kv_ir_search.cpp(1 hunks)docs/src/dev-docs/components-core/index.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit configuration file
- Prefer
false == <expression>rather than!<expression>.
Files:
components/core/src/clp/ffi/ir_stream/search/test/utils.cppcomponents/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cppcomponents/core/src/clp_s/kv_ir_search.cppcomponents/core/src/clp_s/CommandLineArguments.cppcomponents/core/src/clp_s/Utils.cpp
🧠 Learnings (5)
📓 Common learnings
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1122
File: components/core/src/clp/clp/CMakeLists.txt:175-195
Timestamp: 2025-07-23T09:54:45.185Z
Learning: In the CLP project, when reviewing CMakeLists.txt changes that introduce new compression library dependencies (BZip2, LibLZMA, LZ4, ZLIB), the team prefers to address conditional linking improvements in separate PRs rather than expanding the scope of focused migration PRs like the LibArchive task-based installation migration.
📚 Learning: 2025-08-25T07:27:19.449Z
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1262
File: docs/src/dev-guide/components-core/index.md:50-50
Timestamp: 2025-08-25T07:27:19.449Z
Learning: In the CLP project, nlohmann_json version 3.11.3 is installed through their dependency management process, but they don't enforce version constraints in CMake find_package calls - they control versions through installation rather than CMake version pinning.
Applied to files:
docs/src/dev-docs/components-core/index.md
📚 Learning: 2024-11-01T03:26:26.386Z
Learnt from: LinZhihao-723
PR: y-scope/clp#570
File: components/core/tests/test-ir_encoding_methods.cpp:376-399
Timestamp: 2024-11-01T03:26:26.386Z
Learning: In the test code (`components/core/tests/test-ir_encoding_methods.cpp`), exception handling for `msgpack::unpack` can be omitted because the Catch2 testing framework captures exceptions if they occur.
Applied to files:
components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp
📚 Learning: 2024-10-14T03:42:10.355Z
Learnt from: LinZhihao-723
PR: y-scope/clp#558
File: components/core/tests/test-ffi_KeyValuePairLogEvent.cpp:14-14
Timestamp: 2024-10-14T03:42:10.355Z
Learning: In the file `components/core/tests/test-ffi_KeyValuePairLogEvent.cpp`, including `<json/single_include/nlohmann/json.hpp>` is consistent with the project's coding standards.
Applied to files:
components/core/src/clp_s/kv_ir_search.cpp
📚 Learning: 2025-01-30T19:26:33.869Z
Learnt from: davemarco
PR: y-scope/clp#700
File: components/core/src/clp/streaming_archive/ArchiveMetadata.hpp:153-155
Timestamp: 2025-01-30T19:26:33.869Z
Learning: When working with constexpr strings (string literals with static storage duration), std::string_view is the preferred choice for member variables as it's more efficient and safe, avoiding unnecessary memory allocations.
Applied to files:
components/core/src/clp_s/kv_ir_search.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). (16)
- GitHub Check: musllinux_1_2-x86_64-static-linked-bins
- GitHub Check: ubuntu-jammy-lint
- GitHub Check: package-image
- GitHub Check: manylinux_2_28-x86_64-static-linked-bins
- GitHub Check: musllinux_1_2-x86_64-dynamic-linked-bins
- GitHub Check: ubuntu-jammy-static-linked-bins
- GitHub Check: centos-stream-9-dynamic-linked-bins
- GitHub Check: ubuntu-jammy-dynamic-linked-bins
- GitHub Check: manylinux_2_28-x86_64-dynamic-linked-bins
- GitHub Check: centos-stream-9-static-linked-bins
- GitHub Check: build-macos (macos-15, false)
- GitHub Check: build-macos (macos-15, true)
- GitHub Check: build-macos (macos-14, false)
- GitHub Check: build-macos (macos-14, true)
- GitHub Check: lint-check (macos-15)
- GitHub Check: lint-check (ubuntu-24.04)
🔇 Additional comments (6)
components/core/src/clp_s/Utils.cpp (1)
10-10: Use of<fmt/format.h>is correct hereThis TU only uses
fmt::format;fmt/format.his the right include for fmt 11.x. No further changes needed.components/core/src/clp_s/CommandLineArguments.cpp (1)
8-8: Header update aligns with fmt 11.x APIsOnly
fmt::formatis used;<fmt/format.h>is the appropriate include. Looks good.components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp (1)
14-14: Ranges include justified byfmt::join; redundantfmt/core.his gone
fmt::joinrequires<fmt/ranges.h>in fmt 11.x, andfmt::formatis covered by<fmt/format.h>. This fixes earlier redundancy and is correct.components/core/src/clp_s/kv_ir_search.cpp (1)
11-11: Correct fmt header forfmt::formatThis TU formats to stdout only;
<fmt/format.h>is the right include for fmt 11.x.docs/src/dev-docs/components-core/index.md (1)
46-56: Verify deps alignment and remove obsolete fmt header
taskfiles/deps/main.yamlcorrectly pins fmt v11.2.0, spdlog v1.15.3, and log-surgeon 831a9f9 (matches docs).- Remove or replace the obsolete
#include <fmt/core.h>incomponents/core/tests/test-FloatFormatEncoding.cpp:9.components/core/src/clp/ffi/ir_stream/search/test/utils.cpp (1)
13-14: Remove obsolete fmt/core.h include in test-FloatFormatEncoding.cpp
Found a leftover#include <fmt/core.h>at line 9 incomponents/core/tests/test-FloatFormatEncoding.cpp; replace it with<fmt/format.h>(and<fmt/ranges.h>if you use ranges utilities) to complete the fmt 11 migration.⛔ Skipped due to learnings
Learnt from: LinZhihao-723 PR: y-scope/clp#570 File: components/core/tests/test-ir_encoding_methods.cpp:376-399 Timestamp: 2024-11-01T03:26:26.386Z Learning: In the test code (`components/core/tests/test-ir_encoding_methods.cpp`), exception handling for `msgpack::unpack` can be omitted because the Catch2 testing framework captures exceptions if they occur.Learnt from: LinZhihao-723 PR: y-scope/clp#558 File: components/core/tests/test-ffi_KeyValuePairLogEvent.cpp:14-14 Timestamp: 2024-10-14T03:42:10.355Z Learning: In the file `components/core/tests/test-ffi_KeyValuePairLogEvent.cpp`, including `<json/single_include/nlohmann/json.hpp>` is consistent with the project's coding standards.
There was a problem hiding this comment.
Looks correct. Docs are now consistent with the changes and it has all the version updates needed for #1313.
Bill-hbrhbr
changed the title
fmtlib to 11.2.0, spdlog to 1.15.3, and log-surgeon to the newest version.
There was a problem hiding this comment.
Deferring to @gibber9809 & @SharafMohamed's reviews.
For the PR title, how about:
build(deps): Bump fmtlib to v11.2.0, spdlog to v1.15.3, and log-surgeon to y-scope/log-surgeon@a722d07.
@gibber9809, don't think we need to add backticks around the library names and versions since it shouldn't be ambiguous what we mean with each token. Let me know if you disagree.
No, I'm fine with this title. |
Bill-hbrhbr
changed the title
fmtlib to 11.2.0, spdlog to 1.15.3, and log-surgeon to the newest version.
Description
As a prerequisite of upgrading clang-tidy from v19 to v20, we need to bump the versions of
fmtlibto satisfy lint requirements.Due to the conflicts between the old and the new
fmt, other libraries that are dependent onfmtlibalso need to be updated/bumped:spdloglog-surgeonBumps
fmtlibversion and cleans up fmt headers. In11.2.0:<fmt/format.h>brings infmt::formatfmt::memory_bufferfmt::to_stringfmt::vformat<fmt/base.h>brings infmt::format_tofmt::make_format_args<fmt/ranges.h>brings infmt::join<fmt/core.h>is obsolete.Note:
fmtversion of10.1.1because they are willing to create a separate branch that uses the most up-to-date deps versions.log_surgeon::Query. #1313Checklist
breaking change.
Validation performed
Summary by CodeRabbit