Skip to content

refactor(clp): Modularize ReaderInterface and WriterInterface into library io_interface and update all references. - #724

Closed
Bill-hbrhbr wants to merge 4 commits into
y-scope:mainfrom
Bill-hbrhbr:package-io-interface
Closed

refactor(clp): Modularize ReaderInterface and WriterInterface into library io_interface and update all references.#724
Bill-hbrhbr wants to merge 4 commits into
y-scope:mainfrom
Bill-hbrhbr:package-io-interface

Conversation

@Bill-hbrhbr

@Bill-hbrhbr Bill-hbrhbr commented Feb 19, 2025

Copy link
Copy Markdown
Contributor

Description

  • Move ReaderInterface.*pp and WriterInterface.*pp into a subdirectory folder components/core/src/clp/io_interface and supply a CMakeLists.txt to make it into a library target
    • io_interface will be moved into ystdlib-cpp in the near future
  • Update the namespaces to reflect the nesting:
    • clp::ReaderInterface into clp::io_interface::ReaderInterface
    • clp::WriterInterface into clp::io_interface::WriterInterface
  • Update all reference sites
    • Change relative include paths into `#include <io_interface/*Interface.hpp>
    • Change all namespaces to either clp::io_interface::*Interface or ::clp::io_interface::*Interface for code that reside in clp namespace
      • This is to avoid error msg clp::clp::io_interface not found.
      • Use fully-qualified names at all reference sites for easier future find and replace into ystdlib::io_interface::*Interface
    • Removes *Interface.*pp from CMake source files and provide library linking target clp::io_interface
  • Misc:
    • Added 3 missing includes that resulted in compile failures on my system (WSL1 Ubuntu 20.04). These changes are not harmful and will be necessary if the associated files are to go through clang-tidy.
    • There are no changes to *Interface.*pp files except for
      • Header guards
      • One extra level of namespace io_interface

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

  • No functionality changes. All unit tests still pass.

Summary by CodeRabbit

  • Refactor
    • Streamlined the project’s internal architecture and modular interfaces for improved maintainability.
  • Chores
    • Updated build configurations and removed deprecated components to consolidate our dependency management.
  • Tests
    • Adjusted test files to align with the new internal structure, ensuring ongoing stability.

Note: End-user functionality remains unchanged while internal efficiency and organization are enhanced.

@coderabbitai

coderabbitai Bot commented Feb 19, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

This pull request refactors the project’s I/O interface usage by updating include directives, class inheritance, and method signatures to reference the new clp::io_interface namespace. Across many files, the old ReaderInterface and WriterInterface have been replaced with their fully qualified counterparts, the build configuration has been updated to remove the old sources, and new library and alias definitions have been added. No control-flow or functional changes are introduced.

Changes

File(s) Change Summary
components/core/.clang-format Modified IncludeCategories regex to include io_interface.
components/core/CMakeLists.txt, .../clp/clg/CMakeLists.txt, .../clp/clo/CMakeLists.txt, .../clp/clp/CMakeLists.txt, .../clp_s/CMakeLists.txt, .../indexer/CMakeLists.txt Removed old ReaderInterface/WriterInterface source files; added new subdirectory and dependency on clp::io_interface.
components/core/src/clp/BoundedReader.hpp, BufferReader.hpp, BufferedFileReader.hpp, FileDescriptorReader.hpp, FileReader.hpp Updated class inheritance and header includes to use ::clp::io_interface::ReaderInterface.
components/core/src/clp/FileWriter.hpp, and files in components/core/src/clp/streaming_compression/ (including Compressor, Decompressor in various subdirectories) Updated class inheritance and method signatures to use ::clp::io_interface::WriterInterface with modified include paths.
components/core/src/clp/ffi/ir_stream/* Refactored deserialization method signatures and utilities to accept ::clp::io_interface::ReaderInterface& and adjusted includes accordingly.
components/core/src/clp/io_interface/ReaderInterface.{cpp,hpp}, WriterInterface.{cpp,hpp} Introduced new library files with updated namespace declarations and include guards to clp::io_interface.
components/core/src/clp_s/* (e.g., ArchiveReaderAdaptor, JsonFileIterator, PackedStreamReader, ReaderUtils, ZstdDecompressor, etc.) Updated file inclusions and type references from clp::ReaderInterface to clp::io_interface::ReaderInterface.
components/core/tests/* Modified test files to use the new ReaderInterface from io_interface instead of the old namespace.
components/core/src/glt/GlobalMetadataDBConfig.hpp Added the inclusion of <cstdint>.

Sequence Diagram(s)

(No sequence diagrams generated as the changes focus on refactoring and namespace updates rather than control-flow modifications.)

Possibly related PRs

Suggested reviewers

  • kirkrodrigues
  • LinZhihao-723
  • gibber9809
✨ Finishing Touches
  • 📝 Generate Docstrings (Beta)

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ 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.
    • Generate unit testing code for this file.
    • 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. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • 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 src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

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.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • 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 changed the title Package Reader/Writer Interface into a lib refactor(clp): Modularize ReaderInterface and WriterInterface into library io_interface and update all references. Feb 20, 2025
@Bill-hbrhbr
Bill-hbrhbr marked this pull request as ready for review February 20, 2025 21:17
@Bill-hbrhbr
Bill-hbrhbr requested a review from a team as a code owner February 20, 2025 21:17

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🔭 Outside diff range comments (1)
components/core/src/clp/clp/FileCompressor.cpp (1)

129-130: 🛠️ Refactor suggestion

Follow coding guidelines for boolean expressions.

According to the coding guidelines, prefer false == <expression> rather than !<expression>.

Apply this diff:

-    if (auto error_code = m_file_reader.try_refill_buffer_if_empty();
-        ErrorCode_Success != error_code && ErrorCode_EndOfFile != error_code)
+    if (auto error_code = m_file_reader.try_refill_buffer_if_empty();
+        false == (ErrorCode_Success == error_code || ErrorCode_EndOfFile == error_code))
🧹 Nitpick comments (3)
components/core/src/clp/clp/FileCompressor.cpp (1)

256-258: Improve readability of the condition.

The condition can be simplified by removing unnecessary parentheses and aligning the comparison operator.

Apply this diff:

-        } else if ((archive_writer.get_file().get_encoded_size_in_bytes()
-                    >= target_encoded_file_size))
+        } else if (archive_writer.get_file().get_encoded_size_in_bytes()
+                   >= target_encoded_file_size)
components/core/src/clp/LogSurgeonReader.cpp (1)

10-10: Update condition to follow coding guidelines.

The condition should use false == instead of ! to align with coding guidelines.

-        if (read_to == 0) {
+        if (false == (read_to != 0)) {
components/core/src/clp/ffi/ir_stream/decoding_methods.cpp (1)

505-505: Consider addressing the TODO comment.

The TODO comment indicates that version parsing needs to be implemented properly.

Would you like me to help implement a proper version parser or create an issue to track this task?

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2aa5c5c and 4dff345.

📒 Files selected for processing (67)
  • components/core/.clang-format (1 hunks)
  • components/core/CMakeLists.txt (2 hunks)
  • components/core/src/clp/BoundedReader.hpp (3 hunks)
  • components/core/src/clp/BufferReader.hpp (1 hunks)
  • components/core/src/clp/BufferedFileReader.hpp (2 hunks)
  • components/core/src/clp/FileDescriptorReader.hpp (2 hunks)
  • components/core/src/clp/FileReader.hpp (1 hunks)
  • components/core/src/clp/FileWriter.hpp (1 hunks)
  • components/core/src/clp/GlobalMetadataDBConfig.hpp (1 hunks)
  • components/core/src/clp/LibarchiveFileReader.hpp (1 hunks)
  • components/core/src/clp/LibarchiveReader.cpp (1 hunks)
  • components/core/src/clp/LibarchiveReader.hpp (3 hunks)
  • components/core/src/clp/LogSurgeonReader.cpp (1 hunks)
  • components/core/src/clp/LogSurgeonReader.hpp (1 hunks)
  • components/core/src/clp/MessageParser.cpp (2 hunks)
  • components/core/src/clp/MessageParser.hpp (2 hunks)
  • components/core/src/clp/NetworkReader.hpp (3 hunks)
  • components/core/src/clp/StringReader.hpp (1 hunks)
  • components/core/src/clp/Utils.cpp (1 hunks)
  • components/core/src/clp/clg/CMakeLists.txt (1 hunks)
  • components/core/src/clp/clo/CMakeLists.txt (1 hunks)
  • components/core/src/clp/clp/CMakeLists.txt (1 hunks)
  • components/core/src/clp/clp/FileCompressor.cpp (2 hunks)
  • components/core/src/clp/clp/FileCompressor.hpp (4 hunks)
  • components/core/src/clp/ffi/ir_stream/Deserializer.hpp (5 hunks)
  • components/core/src/clp/ffi/ir_stream/decoding_methods.cpp (1 hunks)
  • components/core/src/clp/ffi/ir_stream/decoding_methods.hpp (10 hunks)
  • components/core/src/clp/ffi/ir_stream/ir_unit_deserialization_methods.cpp (2 hunks)
  • components/core/src/clp/ffi/ir_stream/ir_unit_deserialization_methods.hpp (4 hunks)
  • components/core/src/clp/ffi/ir_stream/utils.hpp (5 hunks)
  • components/core/src/clp/io_interface/CMakeLists.txt (1 hunks)
  • components/core/src/clp/io_interface/ReaderInterface.cpp (2 hunks)
  • components/core/src/clp/io_interface/ReaderInterface.hpp (2 hunks)
  • components/core/src/clp/io_interface/WriterInterface.cpp (2 hunks)
  • components/core/src/clp/io_interface/WriterInterface.hpp (2 hunks)
  • components/core/src/clp/ir/LogEventDeserializer.cpp (2 hunks)
  • components/core/src/clp/ir/LogEventDeserializer.hpp (4 hunks)
  • components/core/src/clp/make_dictionaries_readable/CMakeLists.txt (1 hunks)
  • components/core/src/clp/streaming_compression/Compressor.hpp (2 hunks)
  • components/core/src/clp/streaming_compression/Decompressor.hpp (2 hunks)
  • components/core/src/clp/streaming_compression/lzma/Compressor.cpp (1 hunks)
  • components/core/src/clp/streaming_compression/lzma/Compressor.hpp (4 hunks)
  • components/core/src/clp/streaming_compression/lzma/Decompressor.cpp (2 hunks)
  • components/core/src/clp/streaming_compression/lzma/Decompressor.hpp (2 hunks)
  • components/core/src/clp/streaming_compression/passthrough/Compressor.cpp (2 hunks)
  • components/core/src/clp/streaming_compression/passthrough/Compressor.hpp (2 hunks)
  • components/core/src/clp/streaming_compression/passthrough/Decompressor.hpp (3 hunks)
  • components/core/src/clp/streaming_compression/zstd/Compressor.cpp (2 hunks)
  • components/core/src/clp/streaming_compression/zstd/Compressor.hpp (4 hunks)
  • components/core/src/clp/streaming_compression/zstd/Decompressor.hpp (3 hunks)
  • components/core/src/clp_s/ArchiveReaderAdaptor.cpp (6 hunks)
  • components/core/src/clp_s/ArchiveReaderAdaptor.hpp (5 hunks)
  • components/core/src/clp_s/CMakeLists.txt (1 hunks)
  • components/core/src/clp_s/Decompressor.hpp (2 hunks)
  • components/core/src/clp_s/JsonFileIterator.cpp (1 hunks)
  • components/core/src/clp_s/JsonFileIterator.hpp (3 hunks)
  • components/core/src/clp_s/JsonParser.cpp (0 hunks)
  • components/core/src/clp_s/PackedStreamReader.hpp (2 hunks)
  • components/core/src/clp_s/ReaderUtils.cpp (4 hunks)
  • components/core/src/clp_s/ReaderUtils.hpp (2 hunks)
  • components/core/src/clp_s/ZstdDecompressor.cpp (2 hunks)
  • components/core/src/clp_s/ZstdDecompressor.hpp (3 hunks)
  • components/core/src/clp_s/indexer/CMakeLists.txt (1 hunks)
  • components/core/src/glt/GlobalMetadataDBConfig.hpp (1 hunks)
  • components/core/tests/test-FileDescriptorReader.cpp (3 hunks)
  • components/core/tests/test-MemoryMappedFile.cpp (1 hunks)
  • components/core/tests/test-NetworkReader.cpp (4 hunks)
💤 Files with no reviewable changes (1)
  • components/core/src/clp_s/JsonParser.cpp
✅ Files skipped from review due to trivial changes (9)
  • components/core/src/clp/GlobalMetadataDBConfig.hpp
  • components/core/src/glt/GlobalMetadataDBConfig.hpp
  • components/core/src/clp/Utils.cpp
  • components/core/src/clp/io_interface/ReaderInterface.cpp
  • components/core/src/clp/ffi/ir_stream/ir_unit_deserialization_methods.cpp
  • components/core/.clang-format
  • components/core/src/clp/io_interface/WriterInterface.hpp
  • components/core/src/clp_s/PackedStreamReader.hpp
  • components/core/src/clp/io_interface/ReaderInterface.hpp
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ...

**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}: - Prefer false == <expression> rather than !<expression>.

  • components/core/src/clp/LogSurgeonReader.cpp
  • components/core/tests/test-MemoryMappedFile.cpp
  • components/core/src/clp/FileWriter.hpp
  • components/core/src/clp/NetworkReader.hpp
  • components/core/src/clp_s/ZstdDecompressor.cpp
  • components/core/tests/test-FileDescriptorReader.cpp
  • components/core/src/clp/BufferedFileReader.hpp
  • components/core/src/clp/StringReader.hpp
  • components/core/src/clp/streaming_compression/lzma/Compressor.cpp
  • components/core/src/clp/MessageParser.hpp
  • components/core/src/clp/LibarchiveReader.cpp
  • components/core/src/clp/LibarchiveReader.hpp
  • components/core/src/clp/streaming_compression/zstd/Compressor.cpp
  • components/core/src/clp/streaming_compression/lzma/Decompressor.cpp
  • components/core/src/clp/LogSurgeonReader.hpp
  • components/core/src/clp_s/JsonFileIterator.cpp
  • components/core/src/clp/BufferReader.hpp
  • components/core/src/clp/streaming_compression/lzma/Decompressor.hpp
  • components/core/src/clp/io_interface/WriterInterface.cpp
  • components/core/src/clp/streaming_compression/passthrough/Compressor.hpp
  • components/core/src/clp/MessageParser.cpp
  • components/core/src/clp_s/Decompressor.hpp
  • components/core/src/clp_s/ReaderUtils.hpp
  • components/core/src/clp/streaming_compression/Compressor.hpp
  • components/core/src/clp/ffi/ir_stream/utils.hpp
  • components/core/src/clp/FileReader.hpp
  • components/core/tests/test-NetworkReader.cpp
  • components/core/src/clp_s/ZstdDecompressor.hpp
  • components/core/src/clp_s/ArchiveReaderAdaptor.cpp
  • components/core/src/clp/LibarchiveFileReader.hpp
  • components/core/src/clp/ir/LogEventDeserializer.cpp
  • components/core/src/clp_s/JsonFileIterator.hpp
  • components/core/src/clp/clp/FileCompressor.hpp
  • components/core/src/clp/clp/FileCompressor.cpp
  • components/core/src/clp/streaming_compression/passthrough/Compressor.cpp
  • components/core/src/clp/streaming_compression/passthrough/Decompressor.hpp
  • components/core/src/clp/streaming_compression/Decompressor.hpp
  • components/core/src/clp/ffi/ir_stream/Deserializer.hpp
  • components/core/src/clp/FileDescriptorReader.hpp
  • components/core/src/clp/streaming_compression/zstd/Decompressor.hpp
  • components/core/src/clp/streaming_compression/zstd/Compressor.hpp
  • components/core/src/clp/ffi/ir_stream/decoding_methods.hpp
  • components/core/src/clp/ffi/ir_stream/ir_unit_deserialization_methods.hpp
  • components/core/src/clp/BoundedReader.hpp
  • components/core/src/clp/streaming_compression/lzma/Compressor.hpp
  • components/core/src/clp_s/ReaderUtils.cpp
  • components/core/src/clp/ir/LogEventDeserializer.hpp
  • components/core/src/clp/ffi/ir_stream/decoding_methods.cpp
  • components/core/src/clp_s/ArchiveReaderAdaptor.hpp
🧠 Learnings (3)
components/core/src/clp/clp/FileCompressor.hpp (1)
Learnt from: haiqi96
PR: y-scope/clp#523
File: components/core/src/clp/clp/FileCompressor.hpp:58-78
Timestamp: 2024-11-10T16:46:53.300Z
Learning: When reviewing legacy code refactors, avoid suggesting changes that would extend the scope of the PR.
components/core/src/clp/ffi/ir_stream/Deserializer.hpp (1)
Learnt from: LinZhihao-723
PR: y-scope/clp#549
File: components/core/src/clp/ffi/ir_stream/Deserializer.hpp:209-210
Timestamp: 2024-11-10T16:46:53.300Z
Learning: Until #486 is merged with customized error code support, introducing a layer from `IRErrorCode` to `std::errc` is unnecessary.
components/core/src/clp/BoundedReader.hpp (1)
Learnt from: gibber9809
PR: y-scope/clp#624
File: components/core/src/clp/BoundedReader.hpp:25-27
Timestamp: 2024-12-10T16:56:33.545Z
Learning: In the CLP project, within constructors like `BoundedReader` in `components/core/src/clp/BoundedReader.hpp`, it's acceptable to call methods like `get_pos()` that may throw exceptions without handling potential errors via `try_get_pos()`, since the constructor itself handles errors by throwing.
🔇 Additional comments (120)
components/core/src/clp/clp/FileCompressor.hpp (2)

7-7: LGTM! Include path updated to reflect the new library structure.

The include directive has been correctly updated to use the new io_interface library path.


75-75: LGTM! Parameter types consistently updated to use the new namespace.

The ReaderInterface parameter types have been correctly updated to use the fully qualified name ::clp::io_interface::ReaderInterface across all methods, aligning with the namespace reorganization.

Also applies to: 85-85, 125-125

components/core/src/clp/io_interface/WriterInterface.cpp (2)

5-5: LGTM! Namespace changes align with modularization objectives.

The namespace changes correctly implement the planned modularization of I/O interfaces into a separate library component.

Also applies to: 37-37


16-16: LGTM! Consistent and guideline-compliant error handling.

The error handling pattern consistently follows the coding guideline to prefer false == <expression> over !<expression> across all methods.

Also applies to: 23-23, 31-31

components/core/src/clp/streaming_compression/zstd/Decompressor.hpp (3)

8-8: LGTM! Include path correctly updated for the new library structure.

The include path change appropriately reflects the movement of ReaderInterface to the new io_interface library.


83-84: LGTM! Method signature correctly updated with fully qualified namespace.

The use of ::clp::io_interface::ReaderInterface properly reflects the new namespace structure.


137-137: LGTM! Member variable type correctly updated with fully qualified namespace.

The use of ::clp::io_interface::ReaderInterface properly reflects the new namespace structure.

components/core/src/clp/ffi/ir_stream/Deserializer.hpp (2)

12-12: LGTM! Include path updated correctly.

The include path has been updated to use the new library structure, which aligns with the PR's objective of modularizing the interfaces.


52-54: LGTM! Namespace changes are consistent.

The parameter types have been correctly updated to use the new clp::io_interface namespace, maintaining consistency with the PR's objective of reorganizing the code structure.

Also applies to: 101-101, 138-141, 187-189

components/core/src/clp/clp/FileCompressor.cpp (1)

10-10: LGTM! The namespace changes align with the PR objectives.

The include directive and using declaration for ReaderInterface from the new io_interface namespace are correctly placed and follow the established pattern.

Also applies to: 23-23

components/core/src/clp/LogSurgeonReader.hpp (1)

4-4: LGTM! The namespace qualifiers and include path have been updated correctly.

The changes consistently reflect the new modular structure by:

  1. Including from the new io_interface library
  2. Using fully qualified names for ReaderInterface

Also applies to: 13-13, 16-16

components/core/src/clp/LogSurgeonReader.cpp (1)

3-4: LGTM! The namespace qualifiers and include path have been updated correctly.

The changes consistently reflect the new modular structure by:

  1. Including from the new io_interface library
  2. Using fully qualified names for ReaderInterface

Also applies to: 6-6

components/core/src/clp/streaming_compression/passthrough/Compressor.cpp (1)

5-6: LGTM! The namespace qualifiers and include path have been updated correctly.

The changes consistently reflect the new modular structure by:

  1. Including from the new io_interface library
  2. Using fully qualified names for WriterInterface

Also applies to: 47-47

components/core/src/clp/streaming_compression/lzma/Decompressor.cpp (1)

5-6: LGTM! The namespace qualifiers and include path have been updated correctly.

The changes consistently reflect the new modular structure by:

  1. Including from the new io_interface library
  2. Using fully qualified names for ReaderInterface

Also applies to: 35-35

components/core/src/clp_s/Decompressor.hpp (2)

8-8: LGTM! Include path updated to reflect new module structure.

The include path has been correctly updated to use the new io_interface module.


60-60: LGTM! Method signature updated to use fully qualified namespace.

The parameter type has been correctly updated to use clp::io_interface::ReaderInterface.

components/core/tests/test-MemoryMappedFile.cpp (2)

8-8: LGTM! Include path and using directive added correctly.

The include path has been updated to use the new io_interface module, and the using directive helps reduce namespace verbosity.

Also applies to: 13-13


21-21: LGTM! Function signatures updated consistently.

The function declaration and definition have been correctly updated to use the simplified ReaderInterface type through the using directive.

Also applies to: 25-25

components/core/src/clp/streaming_compression/Compressor.hpp (2)

8-8: LGTM! Include path updated to reflect new module structure.

The include path has been correctly updated to use the new io_interface module.


17-17: LGTM! Base class and method signature updated consistently.

The base class inheritance and method signature have been correctly updated to use the fully qualified ::clp::io_interface::WriterInterface type.

Also applies to: 75-75

components/core/src/clp/MessageParser.hpp (2)

6-6: LGTM! Include path updated to reflect new module structure.

The include path has been correctly updated to use the new io_interface module.


57-61: LGTM! Method signature updated to use fully qualified namespace.

The parameter type has been correctly updated to use ::clp::io_interface::ReaderInterface.

components/core/src/clp/streaming_compression/Decompressor.hpp (3)

6-6: LGTM! Include path updated correctly.

The include path has been updated to use the new io_interface library location.


12-12: LGTM! Base class reference updated correctly.

The base class now uses the fully qualified name ::clp::io_interface::ReaderInterface.


55-57: LGTM! Method parameter type updated correctly.

The parameter type now uses the fully qualified name ::clp::io_interface::ReaderInterface.

components/core/src/clp/streaming_compression/passthrough/Compressor.hpp (3)

6-6: LGTM! Include path updated correctly.

The include path has been updated to use the new io_interface library location.


76-76: LGTM! Method parameter type updated correctly.

The parameter type now uses the fully qualified name ::clp::io_interface::WriterInterface.


80-80: LGTM! Member variable type updated correctly.

The member variable type now uses the fully qualified name ::clp::io_interface::WriterInterface.

components/core/src/clp/FileWriter.hpp (2)

7-7: LGTM! Include path updated correctly.

The include path has been updated to use the new io_interface library location.


13-13: LGTM! Base class reference updated correctly.

The base class now uses the fully qualified name ::clp::io_interface::WriterInterface.

components/core/tests/test-FileDescriptorReader.cpp (3)

8-8: LGTM! Include path updated correctly.

The include path has been updated to use the new io_interface library location.


15-15: LGTM! Using directive added correctly.

The using directive has been added to simplify the usage of ReaderInterface from the new namespace.


30-31: LGTM! Function parameter type updated correctly.

The parameter type has been updated to use ReaderInterface from the new namespace in both function declarations.

Also applies to: 43-43

components/core/src/clp/FileReader.hpp (2)

9-9: LGTM! Include path updated correctly.

The include path has been updated to use the new modular structure, which aligns with the PR objectives.


16-16: LGTM! Class inheritance updated correctly.

The inheritance now uses the fully qualified namespace ::clp::io_interface::ReaderInterface, which prevents any ambiguity in namespace resolution.

components/core/src/clp/StringReader.hpp (2)

7-7: LGTM! Include path updated correctly.

The include path has been updated to use the new modular structure, maintaining consistency across the codebase.


14-14: LGTM! Class inheritance updated correctly.

The inheritance now uses the fully qualified namespace ::clp::io_interface::ReaderInterface, maintaining consistency with other reader implementations.

components/core/src/clp_s/ReaderUtils.hpp (2)

6-6: LGTM! Include path updated correctly.

The include path has been updated to use the new modular structure, maintaining consistency across the codebase.


77-78: LGTM! Return type updated correctly.

The return type now uses the fully qualified namespace clp::io_interface::ReaderInterface, and the documentation has been updated accordingly.

components/core/src/clp/streaming_compression/zstd/Compressor.hpp (4)

6-6: LGTM! Include path updated correctly.

The include path has been updated to use the new modular structure, maintaining consistency across the codebase.


77-77: LGTM! Parameter type updated correctly.

The parameter type now uses the fully qualified namespace ::clp::io_interface::WriterInterface, maintaining consistency with other implementations.


86-86: LGTM! Parameter type updated correctly.

The parameter type now uses the fully qualified namespace ::clp::io_interface::WriterInterface, maintaining consistency with other implementations.


95-95: LGTM! Member variable type updated correctly.

The member variable type now uses the fully qualified namespace ::clp::io_interface::WriterInterface, maintaining consistency with other implementations.

components/core/src/clp_s/JsonFileIterator.hpp (1)

4-4: LGTM! Namespace and include path updates are correct.

The changes properly reflect the modularization of ReaderInterface into the io_interface library:

  • Include path updated to use the new library path
  • Type references properly qualified with the new namespace

Also applies to: 24-24, 82-82

components/core/src/clp/ir/LogEventDeserializer.hpp (1)

6-6: LGTM! Namespace and include path updates are correct.

The changes properly reflect the modularization of ReaderInterface into the io_interface library:

  • Include path updated to use the new library path
  • Type references properly qualified with global scope operator and new namespace

Also applies to: 37-37, 69-69, 73-73, 86-86

components/core/src/clp/BoundedReader.hpp (1)

7-8: LGTM! Namespace, include path, and error handling updates are correct.

The changes properly reflect the modularization of ReaderInterface into the io_interface library:

  • Include path updated to use the new library path
  • Type references properly qualified with global scope operator and new namespace
  • Error handling correctly updated to use fully qualified names

Also applies to: 21-21, 24-24, 28-32, 36-40, 92-92

components/core/src/clp/ffi/ir_stream/ir_unit_deserialization_methods.hpp (1)

9-9: LGTM! Namespace and include path updates are correct.

The changes properly reflect the modularization of ReaderInterface into the io_interface library:

  • Include path updated to use the new library path
  • Type references properly qualified with new namespace

Also applies to: 43-43, 55-55, 77-77

components/core/src/clp/FileDescriptorReader.hpp (1)

11-11: LGTM! Clean namespace refactoring.

The changes correctly update the include path and class inheritance to use the new io_interface namespace, aligning with the PR objectives.

Also applies to: 27-27

components/core/src/clp/BufferReader.hpp (1)

4-4: LGTM! Clean namespace refactoring.

The changes correctly update the include path and class inheritance to use the new io_interface namespace, aligning with the PR objectives.

Also applies to: 10-10

components/core/src/clp/streaming_compression/lzma/Decompressor.hpp (1)

6-6: LGTM! Clean namespace refactoring.

The changes correctly update the include path and method parameter type to use the new io_interface namespace, aligning with the PR objectives.

Also applies to: 85-86

components/core/src/clp/LibarchiveFileReader.hpp (1)

8-8: LGTM! Clean namespace refactoring.

The changes correctly update the include path and class inheritance to use the new io_interface namespace, aligning with the PR objectives.

Also applies to: 17-17

components/core/src/clp/streaming_compression/passthrough/Decompressor.hpp (3)

4-4: LGTM! Include path updated correctly.

The include path has been updated to use the new io_interface library location.


78-79: LGTM! Method signature updated correctly.

The open method signature has been updated to use the fully qualified namespace for ReaderInterface.


106-106: LGTM! Member variable type updated correctly.

The member variable type has been updated to use the fully qualified namespace for ReaderInterface.

components/core/src/clp/LibarchiveReader.hpp (3)

8-8: LGTM! Include path updated correctly.

The include path has been updated to use the new io_interface library location.


47-50: LGTM! Method signature updated correctly.

The try_open method signature has been updated to use the fully qualified namespace for ReaderInterface.


151-151: LGTM! Member variable type updated correctly.

The member variable type has been updated to use the fully qualified namespace for ReaderInterface.

components/core/src/clp/MessageParser.cpp (2)

3-4: LGTM! Include directive added correctly.

The include directive for ReaderInterface has been added with proper spacing.


52-52: LGTM! Method parameter type updated correctly.

The parse_next_message method parameter type has been updated to use the fully qualified namespace for ReaderInterface.

components/core/src/clp_s/ZstdDecompressor.hpp (3)

10-10: LGTM! Include path updated correctly.

The include path has been updated to use the new io_interface library location.


46-47: LGTM! Method signature updated correctly.

The open method signature has been updated to use the fully qualified namespace for ReaderInterface.


135-135: LGTM! Member variable type updated correctly.

The member variable type has been updated to use the fully qualified namespace for ReaderInterface.

components/core/src/clp_s/ArchiveReaderAdaptor.hpp (6)

9-9: LGTM!

The include directive is correctly added to support the new namespace.


50-52: LGTM!

The method signature is correctly updated to use the new namespace.


101-101: LGTM!

The method signature is correctly updated to use the new namespace.


110-112: LGTM!

The method signature is correctly updated to use the new namespace.


120-120: LGTM!

The method signature is correctly updated to use the new namespace.


139-139: LGTM!

The member variable type is correctly updated to use the new namespace.

components/core/src/clp/streaming_compression/zstd/Compressor.cpp (2)

5-5: LGTM!

The include directive is correctly added to support the new namespace.


29-29: LGTM!

The method signature is correctly updated to use the new namespace.

components/core/src/clp_s/JsonFileIterator.cpp (2)

6-6: LGTM!

The include directive is correctly added to support the new namespace.


11-11: LGTM!

The method signature is correctly updated to use the new namespace.

components/core/src/clp/ir/LogEventDeserializer.cpp (4)

5-5: LGTM!

The include directive is correctly added to support the new namespace.


16-16: LGTM!

The using directive is correctly added to support the new namespace.


140-141: LGTM!

The method signature is correctly updated to use the new namespace.


142-143: LGTM!

The method signature is correctly updated to use the new namespace.

components/core/src/clp/LibarchiveReader.cpp (2)

4-4: LGTM! The include path aligns with the new library structure.

The updated include path correctly reflects the new location of the ReaderInterface header in the io_interface library.


10-10: LGTM! The using directive is correctly scoped.

The using directive is appropriately placed and helps maintain code readability by avoiding repetitive namespace qualifications.

components/core/src/clp/streaming_compression/lzma/Compressor.cpp (2)

8-8: LGTM! The include path aligns with the new library structure.

The updated include path correctly reflects the new location of the WriterInterface header in the io_interface library.


17-17: LGTM! The method signature is correctly updated.

The method signature now uses the fully qualified type from the new io_interface library, maintaining consistency with the header file.

components/core/src/clp_s/ReaderUtils.cpp (4)

7-7: LGTM! The include path aligns with the new library structure.

The updated include path correctly reflects the new location of the ReaderInterface header in the io_interface library.


18-18: LGTM! The using directive is correctly scoped.

The using directive is appropriately placed and helps maintain code readability by avoiding repetitive namespace qualifications.


150-150: LGTM! The method signature is correctly updated.

The method signature now uses the type from the new io_interface library, maintaining consistency with the using directive.


195-196: LGTM! The method signature is correctly updated.

The method signature now uses the type from the new io_interface library, maintaining consistency with the using directive.

components/core/src/clp/streaming_compression/lzma/Compressor.hpp (4)

7-7: LGTM! The include path aligns with the new library structure.

The updated include path correctly reflects the new location of the WriterInterface header in the io_interface library.


18-19: LGTM! The comments are correctly updated.

The comments now use the unqualified type name, which improves readability while maintaining accuracy.

Also applies to: 61-62, 216-216


87-87: LGTM! The method signature is correctly updated.

The method signature now uses the fully qualified type from the new io_interface library, maintaining consistency with the implementation.


223-223: LGTM! The member variable type is correctly updated.

The member variable type now uses the fully qualified type from the new io_interface library, maintaining consistency with the method signature.

components/core/src/clp_s/ArchiveReaderAdaptor.cpp (3)

11-11: LGTM! The changes align with the PR objectives.

The include and using directives correctly reflect the modularization of ReaderInterface into the io_interface library.

Also applies to: 23-23


123-127: LGTM! The function signature is correctly updated.

The function signature now uses ReaderInterface from the using directive, maintaining consistency with the PR's modularization objectives.


190-257: LGTM! The function signatures are correctly updated.

The function signatures now use ReaderInterface from the using directive, maintaining consistency with the PR's modularization objectives.

components/core/src/clp/ffi/ir_stream/decoding_methods.hpp (7)

8-9: LGTM! The include directive is correctly updated.

The include directive correctly reflects the modularization of ReaderInterface into the io_interface library.


59-61: LGTM! The function signature is correctly updated.

The function signature now uses the fully qualified ::clp::io_interface::ReaderInterface, maintaining consistency with the PR's modularization objectives.


166-171: LGTM! The function signature is correctly updated.

The function signature now uses the fully qualified ::clp::io_interface::ReaderInterface, maintaining consistency with the PR's modularization objectives.


182-186: LGTM! The function signature is correctly updated.

The function signature now uses the fully qualified ::clp::io_interface::ReaderInterface, maintaining consistency with the PR's modularization objectives.


195-196: LGTM! The function signature is correctly updated.

The function signature now uses the fully qualified ::clp::io_interface::ReaderInterface, maintaining consistency with the PR's modularization objectives.


227-232: LGTM! The function signature is correctly updated.

The function signature now uses the fully qualified ::clp::io_interface::ReaderInterface, maintaining consistency with the PR's modularization objectives.


247-252: LGTM! The function signature is correctly updated.

The function signature now uses the fully qualified ::clp::io_interface::ReaderInterface, maintaining consistency with the PR's modularization objectives.

components/core/src/clp/BufferedFileReader.hpp (2)

10-11: LGTM! The include directive is correctly updated.

The include directive correctly reflects the modularization of ReaderInterface into the io_interface library.


37-37: LGTM! The class inheritance is correctly updated.

The class now inherits from the fully qualified ::clp::io_interface::ReaderInterface, maintaining consistency with the PR's modularization objectives.

components/core/src/clp/ffi/ir_stream/utils.hpp (3)

12-13: LGTM! The include directive is correctly updated.

The include directive correctly reflects the modularization of ReaderInterface into the io_interface library.


52-53: LGTM! The function signature is correctly updated.

The function signature now uses clp::io_interface::ReaderInterface, maintaining consistency with the PR's modularization objectives.


155-172: LGTM! The function signature is correctly updated.

The function signature now uses clp::io_interface::ReaderInterface, maintaining consistency with the PR's modularization objectives.

components/core/tests/test-NetworkReader.cpp (3)

15-15: LGTM! Clean namespace transition.

The include directive and using declaration are correctly updated to use the new io_interface namespace.

Also applies to: 26-26


42-42: LGTM! Function signature updated correctly.

The get_content function signature is properly updated to use the unqualified ReaderInterface type after the using declaration.


72-72: LGTM! Implementation remains consistent.

The function implementation maintains its functionality while using the updated interface type.

components/core/src/clp_s/ZstdDecompressor.cpp (2)

9-9: LGTM! Clean namespace transition.

The include directive and using declaration are correctly updated to use the new io_interface namespace.

Also applies to: 16-16


190-193: LGTM! Method signature updated correctly.

The open method signature is properly updated to use the fully qualified clp::io_interface::ReaderInterface type.

components/core/src/clp/NetworkReader.hpp (2)

21-21: LGTM! Include path updated correctly.

The include directive is properly updated to use the new io_interface path.


39-39: LGTM! Base class reference updated correctly.

The base class is properly updated to use the fully qualified ::clp::io_interface::ReaderInterface name.

components/core/src/clp/ffi/ir_stream/decoding_methods.cpp (1)

9-9: LGTM! Clean namespace transition.

The include directive and using declaration are correctly updated to use the new io_interface namespace.

Also applies to: 16-16

components/core/src/clp/io_interface/CMakeLists.txt (1)

1-18: Effective Modularisation of IO Interfaces.
This new CMake configuration clearly defines the io_interface library by separating header and source lists and establishing a proper alias (clp::io_interface). The include directories and compile feature (cxx_std_20) settings are well configured. Please verify that the relative include (../) correctly targets the intended parent directory.

components/core/src/clp/make_dictionaries_readable/CMakeLists.txt (1)

47-47: Correct Dependency Update for make-dictionaries-readable.
The addition of clp::io_interface in the target link libraries is appropriately implemented. This change neatly replaces the old Reader/Writer interface source files. Ensure that no residual references to the removed interface files remain elsewhere.

components/core/src/clp_s/indexer/CMakeLists.txt (1)

87-87: Accurate Replacement in Indexer Component.
Including clp::io_interface as a dependency correctly reflects the modularised IO interfaces. This update properly removes the legacy references, and it is consistent with the current project-wide refactoring.

components/core/src/clp/clg/CMakeLists.txt (1)

136-136: Consistent Update in CLG Configuration.
The modification that adds clp::io_interface within the target link libraries is executed correctly. The removal of the old ReaderInterface and WriterInterface files aligns with the overall refactor, and the updates maintain build integrity.

components/core/src/clp/clo/CMakeLists.txt (1)

165-165: Proper Dependency Linking for CLO Component.
Adding clp::io_interface to the target link libraries for the clo executable is a precise and necessary update. This change successfully removes the dependency on the previously separate interface files while preserving the build configuration.

components/core/src/clp/clp/CMakeLists.txt (1)

168-178: Modularised IO Interface Linking

The addition of clp::io_interface within the target_link_libraries section accurately reflects the modularisation of the Reader and Writer interfaces. Please verify that the new library target is properly defined and that all references to the old interface files have been updated in the codebase.

components/core/src/clp_s/CMakeLists.txt (1)

252-256: Integrate Modularised IO Interface in CLP-S

The inclusion of clp::io_interface in the linking section for the clp-s executable aligns with the refactor objectives. Ensure that the removal of the now-obsolete interface source files from the CLP_SOURCES list is consistent across the project, and that all include paths and dependencies have been updated accordingly.

components/core/CMakeLists.txt (2)

250-250: Inclusion of the IO Interface Subdirectory

Adding the io_interface subdirectory using add_subdirectory(src/clp/io_interface) modularises the IO components effectively. Please confirm that the new CMake configuration in src/clp/io_interface adheres to the project’s standards and that it compiles without issues.


686-692: Linking IO Interface in Unit Tests

The inclusion of clp::io_interface in the linking setup for the unit test target ensures that tests are built against the updated modular IO interfaces. Verify that all dependent modules compile correctly and that the unit tests continue to pass.

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.

1 participant