Repository navigation
Conversation
Reviewer's GuideThis PR introduces GitHub Actions CI that independently validates the standalone CMake library and the ROS 2 Jazzy packages on pull requests and main-branch pushes, including dependency installation and test execution, and adds badges documenting CI, licensing, ROS distribution, and C++ standard support. Sequence diagram for standalone CI validationsequenceDiagram
participant Actions as GitHub Actions
participant CMake as CMake
participant CTest as CTest
Actions->>CMake: cmake -S piper_cpp -B build
Actions->>CMake: cmake --build build -j
Actions->>CTest: ctest --test-dir build --output-on-failure
CTest-->>Actions: Test result
Sequence diagram for ROS CI validationsequenceDiagram
participant Actions as GitHub Actions
participant ROS as ROS 2 Jazzy container
participant Colcon as Colcon
Actions->>ROS: Install dependencies with rosdep install
Actions->>Colcon: colcon build --packages-up-to piper_cpp_ros piper_cpp_moveit
Actions->>Colcon: colcon test --packages-select piper_cpp piper_cpp_ros piper_cpp_moveit
Colcon-->>Actions: colcon test-result --verbose
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
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 |
There was a problem hiding this comment.
Hey - I've found 5 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path=".github/workflows/ci.yml" line_range="14" />
<code_context>
+ standalone:
+ runs-on: ubuntu-24.04
+ steps:
+ - uses: actions/checkout@v4
+ - name: Install dependencies
+ run: |
</code_context>
<issue_to_address>
**Pull requests expose repository token**
When an untrusted pull request changes build inputs that execute during CI, `actions/checkout@v4` persists its `GITHUB_TOKEN` in the checked-out Git config, and the subsequent CMake and colcon builds execute pull-request-controlled files. A malicious PR can read and exfiltrate the token, using whatever repository permissions are configured, potentially including write access.
Set least-privilege workflow permissions (for example, `contents: read`) and set `persist-credentials: false` on both checkout steps.
Also at `.github/workflows/ci.yml:31`.
</issue_to_address>
### Comment 2
<location path=".github/workflows/ci.yml" line_range="31" />
<code_context>
+ standalone:
+ runs-on: ubuntu-24.04
+ steps:
+ - uses: actions/checkout@v4
+ - name: Install dependencies
+ run: |
</code_context>
<issue_to_address>
**ROS job cannot check out code**
When the ROS job starts in the `ros:jazzy` container, `actions/checkout@v4` runs as a JavaScript action and requires its Node runtime in the job container; the `ros:jazzy` container has no Node installation step, so checkout fails before the workflow's install steps can run.
Run the ROS job in an image that includes the required Node runtime, or use a runner setup that does not execute checkout inside a container without Node.
</issue_to_address>
### Comment 3
<location path=".github/workflows/ci.yml" line_range="41" />
<code_context>
+ apt-get update
+ apt-get install -y --no-install-recommends libsocketcan-dev libgtest-dev
+ rosdep update
+ rosdep install --from-paths src --ignore-src -y --skip-keys piper_description
+ - name: Build
+ shell: bash
</code_context>
<issue_to_address>
**ROS build cannot resolve piper_cpp**
When the ROS job processes this workspace, `rosdep` treats `piper_cpp` as an unresolved dependency because it has no `package.xml`, and colcon cannot discover or build/install this plain CMake project. `piper_cpp_ros` then fails at `find_package(piper_cpp REQUIRED)`, so the ROS job never completes its build or tests.
Make `piper_cpp` recognizable to rosdep and colcon, and build and install it in the workflow with its CMake install prefix available to `piper_cpp_ros`.
Also at `.github/workflows/ci.yml:46`.
</issue_to_address>
### Comment 4
<location path=".github/workflows/ci.yml" line_range="39" />
<code_context>
+ # piper_description is an external runtime dependency not needed to build.
+ run: |
+ apt-get update
+ apt-get install -y --no-install-recommends libsocketcan-dev libgtest-dev
+ rosdep update
+ rosdep install --from-paths src --ignore-src -y --skip-keys piper_description
</code_context>
<issue_to_address>
**ROS tools are missing**
When the `ros:jazzy` image lacks the ROS development tools, the dependency step does not install `python3-rosdep` or `python3-colcon-common-extensions`, so `rosdep update` or later colcon commands fail with command-not-found and the ROS job stops before its checks.
Install `python3-rosdep` and `python3-colcon-common-extensions` before running rosdep or colcon.
</issue_to_address>
### Comment 5
<location path=".github/workflows/ci.yml" line_range="25" />
<code_context>
+ - name: Build
+ run: cmake --build build -j
+ - name: Test
+ run: ctest --test-dir build --output-on-failure
+
+ ros:
</code_context>
<issue_to_address>
**Test steps run no tests**
When the current projects have no registered tests, `ctest` and `colcon test` succeed without executing tests, so CI reports a green test result without checking behavior.
Add and register tests with the relevant test frameworks, and configure the test steps to fail when no tests are found.
Also at `.github/workflows/ci.yml:26`, `.github/workflows/ci.yml:51`.
</issue_to_address>Sourcery assessment
Approval pending. 5 findings to address first.
Blocking findings: .github/workflows/ci.yml:14, .github/workflows/ci.yml:31, .github/workflows/ci.yml:41, .github/workflows/ci.yml:39, .github/workflows/ci.yml:25
Sourcery withdrew this approval because the latest commits introduced blocking findings.
There was a problem hiding this comment.
🟢 Approval recommended
The workflow matches the existing build structure, and no blocking issues were identified.
0 open findings
What changed in this PR
Adds GitHub Actions CI for the standalone C++ library and ROS packages on pull requests and pushes to main.
Changes:
- Adds standalone CMake and ROS 2 Jazzy build/test jobs.
- Adds status and compatibility badges and fixes license links.
| File | Description |
|---|---|
| README.md | Adds project badges and fixes the license link. |
| piper_cpp/README.md | Adds CI, license, and C++17 badges. |
| piper_cpp_ros/README.md | Adds CI, license, ROS, and C++17 badges. |
| piper_cpp_moveit/README.md | Adds badges and fixes the license link. |
| .github/workflows/ci.yml | Defines standalone and ROS build/test jobs. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Builds and tests the repo on every PR and on pushes to main, via two jobs:
in piper_cpp/README.md.
container, followed by colcon test.
Test steps and libgtest-dev are included now so upcoming PRs only need to add tests.
No format check yet: several files currently fail clang-format, so that needs a
separate reformat first.
Also adds CI, licence, ROS distro and C++ standard badges to the top-level and
per-package READMEs.
Summary by Sourcery
Add GitHub Actions CI to build and test the standalone and ROS packages on supported changes.
Enhancements:
CI:
Documentation: