Skip to content

Add project to Fleet Protocol system#9

Merged
koudis merged 1 commit intomainfrom
add_system_name
Oct 17, 2025
Merged

Add project to Fleet Protocol system#9
koudis merged 1 commit intomainfrom
add_system_name

Conversation

@koudis
Copy link
Member

@koudis koudis commented Oct 17, 2025

Summary by CodeRabbit

  • Chores
    • Updated system initialization to include a new required external dependency.

@coderabbitai
Copy link

coderabbitai bot commented Oct 17, 2025

Walkthrough

Added FindPackage requirement for CMLIB with CMCONF component and initialized the CMCONF system with FLEET_PROTOCOL prior to existing storage list definitions. No existing code removed; control flow now ensures CMLIB and CMCONF availability before storage setup.

Changes

Cohort / File(s) Summary
CMake CMLIB dependency setup
CMLibStorage.cmake
Added find_package(CMLIB REQUIRED COMPONENTS CMCONF) and CMCONF system initialization with FLEET_PROTOCOL parameter. Existing STORAGE_LIST definitions preserved.

Sequence Diagram(s)

sequenceDiagram
    participant CMake
    participant CMLIB
    participant CMCONF
    participant Storage as Storage Setup

    CMake->>CMLIB: find_package(REQUIRED COMPONENTS CMCONF)
    activate CMLIB
    CMLIB-->>CMake: Package found
    deactivate CMLIB

    rect rgb(220, 240, 255)
    Note over CMake,CMCONF: New initialization phase
    CMake->>CMCONF: Initialize system with FLEET_PROTOCOL
    activate CMCONF
    CMCONF-->>CMake: Ready
    deactivate CMCONF
    end

    CMake->>Storage: Define STORAGE_LIST & STORAGE_LIST_DEP
    activate Storage
    Storage-->>CMake: Configuration complete
    deactivate Storage
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "Add project to Fleet Protocol system" directly aligns with the main changes in the CMLibStorage.cmake file. The modifications involve adding a FindPackage for CMLIB with CMCONF components and initializing CMCONF with FLEET_PROTOCOL, which represents integrating the project into the Fleet Protocol system. The title is concise, specific enough to convey the primary intent to a teammate reviewing the history, and avoids vague terminology or unnecessary noise.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch add_system_name

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between da71a5c and 7af777d.

📒 Files selected for processing (1)
  • CMLibStorage.cmake (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-04-03T11:53:12.434Z
Learnt from: mikusaq
PR: bringauto/mission-module#23
File: CMLibStorage.cmake:1-3
Timestamp: 2025-04-03T11:53:12.434Z
Learning: In CMLibStorage.cmake, the variable declaration `SET(STORAGE_LIST DEP)` is intentional and should not be changed to `SET(STORAGE_LIST_DEP)` despite appearing to be inconsistent with the later usage.

Applied to files:

  • CMLibStorage.cmake
🔇 Additional comments (2)
CMLibStorage.cmake (2)

3-3: Verify FLEET_PROTOCOL is the correct system identifier for CMCONF_INIT_SYSTEM.

The codebase contains only this single call to CMCONF_INIT_SYSTEM, and no other examples or system identifier definitions are available for comparison. Since CMLIB appears to be internal, verify with the library maintainers or documentation that FLEET_PROTOCOL is the correct parameter and that no additional configuration or error handling is needed after initialization.


1-1: Original review comment is based on an incorrect premise and should be disregarded.

The primary concern—that this "will break builds if CMLIB not installed"—is incorrect. CMakeLists.txt already contains FIND_PACKAGE(CMLIB COMPONENTS CMDEF CMUTIL STORAGE REQUIRED), making CMLIB a pre-existing required dependency. Additionally, README.md documents CMLib as a requirement and provides build instructions with DCMLIB_DIR=<path_to_cmlib>, so developers are already expected to provide CMLIB.

However, there are separate concerns that merit attention:

  • CMLibStorage.cmake is not included or referenced anywhere in the build system
  • The CMCONF component is undocumented and its validity is unclear
  • CMLibStorage.cmake introduces a second FIND_PACKAGE(CMLIB ...) with different components (CMCONF vs. existing CMDEF CMUTIL STORAGE), which could create component conflicts

Likely an incorrect or invalid review comment.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@koudis koudis merged commit a10ac51 into main Oct 17, 2025
1 check was pending
@Nothrax Nothrax deleted the add_system_name branch January 15, 2026 09:45
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