Catkin plugin: Refactor build.
Metadata
Current evaluation
Merged after addressing reviewer feedback on error handling and testing strategy. The update refactored the Catkin plugin build process, simplified build logic, extracted dependency resolution, added a source-space keyword, and achieved 100% unit test coverage.
Suggested action: —
No scores available.
Issue body
This PR contains a number of small refactors:
- Change `env()` to only contain variables for run-time, and document each of them.
- Only obtain dependencies upon `pull()`-- for `build()` they should just be ready to go.
- Extract dependency resolution into a standalone function that can be tested in isolation.
- Change `build()` to be less clever and more simple/verbose/robust.
- Add `source-space` keyword to get rid of custom `build()`.
- Get unit testing coverage of the Catkin plugin to 100%.
Evaluation history
| Date | Model | Scores | Action | Summary |
|---|---|---|---|---|
| qwen/qwen3.6-35b-a3b | — | — | Merged after addressing reviewer feedback on error handling and testing strategy. The update refactored the Catkin plugin build process, simplified build logic, extracted dependency resolution, added a source-space keyword, and achieved 100% unit test coverage. | |
| qwen3.6-35b-a3b-mtp-q6 | — | — | Merged following reviewer approval. Refactored the Catkin plugin build by isolating dependency resolution, simplifying the build method, and achieving 100% test coverage. Clarified environment variables and added a source-space keyword. | |
| qwen3.6-35b-a3b-mtp-q6 | — | — | Merged following reviewer approval. Refactored the Catkin plugin build by isolating dependency resolution, simplifying build logic, adding a source-space keyword, and achieving 100% test coverage. |
Update history
No update history recorded yet.
Related issues
| Issue | Project | State | Summary | Similarity |
|---|---|---|---|---|
| #152 Catkin plugin: Add support for ROS tools. | snapcraft | merged | Merged Catkin plugin updates to properly install ROS packages, include non-source files, and enable ROS tools via an updated wrapper environment. Resolved an xml2-config conflict while planning to remove the redundant roscore plugin. Tests doubled. | |
| #195 Catkin plugin: Improve cmake path rewrite. | snapcraft | merged | Merged updates to the Catkin plugin that replace hard-coded cmake path rewrites with a generic, robust approach. The change modifies two files to correctly redirect ROS package Config.cmake files to the install directory. | |
| #2459 catkin plugin: describe how to build all packages | snapcraft | merged | Merged a documentation update for the catkin plugin explaining how to build all packages. Adds a missing blurb from a previous bug fix. Approved by two reviewers, passed CI, and modified one file. | |
| #1593 catkin tools plugin: add catkin tools support. | snapcraft | merged | Merged a new catkin tools plugin replacing catkin_make. After review, conflict resolution, and adding a beta warning, the changes were approved. Brief testing infrastructure delays preceded successful integration. | |
| #184 Backport to 1.x: Catkin plugin: Refactor build. | snapcraft | merged | Merged backport of PR #175 to the 1.x branch, refactoring the Catkin plugin build across three files. Changes applied directly without reviewer comments or CI checks. | |
| #1743 catkin plugin: support building entire workspace | snapcraft | merged | Merged. The catkin-packages property is now optional. When omitted, the plugin builds the entire workspace. Resolves LP: #1721168. Approved by two reviewers with passing CI checks. | |
| #842 Catkin plugin: Support ROS Kinetic. | snapcraft | merged | Merged. Adds ROS Kinetic support to the Catkin plugin by refactoring source handling for extensibility and mapping ROS to Ubuntu releases. Fixed a fileutils coverage gap during review. Approved and merged. | |
| #1721168 Catkin plugin: support building the entire workspace | snapcraft (launchpad) | closed | Request for Catkin plugin workspace-wide builds was closed without implementation or discussion. No pull requests were opened, indicating the feature request was abandoned. | |
| #163 Catkin plugin: use rosdep for dependency resolution | snapcraft | merged | Merged. Replaced manual package.xml parsing with rosdep for Catkin plugin dependency resolution. Reviewers approved the changes, with minor naming suggestions deferred to a future refactor. | |
| #1130 catkin plugin: produce build-ready staging area. | snapcraft | merged | Merged update to the Catkin plugin generating a build-ready staging area by adjusting ROS files post-build, resolving LP #1663385. Approved, passed CI, and modified two files to prevent future parts from referencing the old install directory. |