feat: add additional package args to ROS test #12
Conversation
WalkthroughAdds multi-package selection to ROS tests, introduces an --include-dependents flag, refactors argument handling to derive packages_to_test and select between --packages-select vs --packages-up-to, and updates result collection to process per-package XMLs when results_dir is provided. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant CLI as ros test (CLI)
participant PS as PackageSelector
participant TR as Test Runner (colcon)
participant RC as ResultsCollector
U->>CLI: ros test --package pkgA [--package pkgB] [--include-dependents] [--results-dir DIR]
CLI->>PS: Determine packages_to_test
alt include_dependents
PS-->>CLI: packages_to_test (changed + dependents)
CLI->>TR: colcon test --packages-up-to packages_to_test
else direct selection
PS-->>CLI: packages_to_test (as provided/changed)
CLI->>TR: colcon test --packages-select packages_to_test
end
note over CLI,TR: Optional: per-package build args when specific packages provided
TR-->>CLI: Test execution completes
opt results_dir provided
loop per package in packages_to_test
CLI->>RC: Collect XML for package
RC-->>CLI: Store in DIR/<package>/*.xml
end
end
CLI-->>U: Exit status and summary
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
platform_cli/groups/ros.py(3 hunks)platform_cli/helpers.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
⚙️ CodeRabbit configuration file
**/*.py: - Use Python 3.8+ features (f-strings, walrus operator, positional-only parameters)
- Use dependency injection patterns - inject dependencies through constructors rather than creating them internally
- Inject configuration and external services (databases, APIs, ROS nodes) rather than hardcoding them
- Design classes with single responsibility principle - each class should have one clear purpose
- Include comprehensive type annotations using typing module (List, Dict, Optional, Union, etc.)
- Prefer built-in functions like
next(),any(),all(),enumerate()over manual loops- Follow ROS2 best practices: use lifecycle nodes, quality of service (QoS) profiles, and parameter declarations
- Design functions and classes to be easily unit testable - avoid tight coupling to ROS nodes
- Use dependency injection patterns to make ROS dependencies mockable in tests
- Implement proper error handling with specific exception types, avoid bare
except:- Use dataclasses or Pydantic models for structured data instead of dictionaries
- Leverage ROS2 latest features: composition, lifecycle management, parameter callbacks
- Add unit tests for complex business logic using pytest, mock ROS dependencies
- Use context managers (
withstatements) for resource management- Prefer list/dict comprehensions and generator expressions over loops where readable
- Follow PEP 8 naming conventions and use meaningful variable names
- Use logging instead of print statements, configure ROS2 logging levels appropriately
- Implement proper cleanup in node destructors and signal handlers
- Use async/await for non-blocking operations where appropriate
- Structure code with clear separation between ROS communication and business logic
- Use piknik's generate_parameter_library for declarative parameters which make unit testing logic that uses parameters easier
- Ensure changes to ROS params, services, publishers and subscribers are documented in the design.md file
Files:
platform_cli/helpers.pyplatform_cli/groups/ros.py
🧬 Code graph analysis (2)
platform_cli/helpers.py (1)
platform_cli/groups/packaging.py (1)
find_packages_with_colcon(25-40)
platform_cli/groups/ros.py (1)
platform_cli/helpers.py (3)
call(280-350)get_changed_files(14-28)find_packages_for_changed_files(31-55)
🪛 Ruff (0.13.1)
platform_cli/helpers.py
27-27: Do not catch blind exception: Exception
(BLE001)
54-54: Do not catch blind exception: Exception
(BLE001)
platform_cli/groups/ros.py
Outdated
| # Section 1: Determine packages to test | ||
| packages_to_test = [] | ||
| if package: | ||
| args_str_test += f" --packages-select {package}" | ||
| args_str_build += f" --package {package}" | ||
| packages_to_test = [package] | ||
| elif changed: | ||
| packages_to_test = get_changed_packages(change_base) | ||
|
|
||
| if not packages_to_test: | ||
| return | ||
|
|
||
| # Section 2: Choose testing strategy | ||
| args_str_test = args_str | ||
| packages_str = " ".join(packages_to_test) | ||
|
|
||
| if include_dependents and changed: | ||
| args_str_test += f" --packages-up-to {packages_str}" | ||
| else: | ||
| args_str_test += f" --packages-select {packages_str}" | ||
|
|
There was a problem hiding this comment.
Default platform ros test now runs zero tests
With the new flow, calling platform ros test without --package or --changed leaves packages_to_test empty, hits the new return, and exits immediately. That regresses the default behaviour from “test everything” to “do nothing”. Worse, any failure in change detection (e.g. missing base branch) also yields an empty list, so even the --changed path silently succeeds without testing. We need to fall back to testing the whole workspace unless the user explicitly asked for the changed-only mode and nothing was found.
Apply this diff:
- # Section 1: Determine packages to test
- packages_to_test = []
- if package:
- packages_to_test = [package]
- elif changed:
- packages_to_test = get_changed_packages(change_base)
-
- if not packages_to_test:
- return
-
- # Section 2: Choose testing strategy
- args_str_test = args_str
- packages_str = " ".join(packages_to_test)
-
- if include_dependents and changed:
- args_str_test += f" --packages-up-to {packages_str}"
- else:
- args_str_test += f" --packages-select {packages_str}"
+ # Section 1: Determine packages to test
+ packages_to_test: List[str] = []
+ if package:
+ packages_to_test = [package]
+ elif changed:
+ packages_to_test = get_changed_packages(change_base)
+ if not packages_to_test:
+ echo("No changed packages detected; skipping tests.", "yellow")
+ return
+
+ # Section 2: Choose testing strategy
+ args_str_test = args_str
+ if packages_to_test:
+ packages_str = " ".join(packages_to_test)
+ if include_dependents and changed:
+ args_str_test += f" --packages-up-to {packages_str}"
+ else:
+ args_str_test += f" --packages-select {packages_str}"(Add from typing import List → from typing import List already exists; no extra imports needed.)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Section 1: Determine packages to test | |
| packages_to_test = [] | |
| if package: | |
| args_str_test += f" --packages-select {package}" | |
| args_str_build += f" --package {package}" | |
| packages_to_test = [package] | |
| elif changed: | |
| packages_to_test = get_changed_packages(change_base) | |
| if not packages_to_test: | |
| return | |
| # Section 2: Choose testing strategy | |
| args_str_test = args_str | |
| packages_str = " ".join(packages_to_test) | |
| if include_dependents and changed: | |
| args_str_test += f" --packages-up-to {packages_str}" | |
| else: | |
| args_str_test += f" --packages-select {packages_str}" | |
| # Section 1: Determine packages to test | |
| packages_to_test: List[str] = [] | |
| if package: | |
| packages_to_test = [package] | |
| elif changed: | |
| packages_to_test = get_changed_packages(change_base) | |
| if not packages_to_test: | |
| echo("No changed packages detected; skipping tests.", "yellow") | |
| return | |
| # Section 2: Choose testing strategy | |
| args_str_test = args_str | |
| if packages_to_test: | |
| packages_str = " ".join(packages_to_test) | |
| if include_dependents and changed: | |
| args_str_test += f" --packages-up-to {packages_str}" | |
| else: | |
| args_str_test += f" --packages-select {packages_str}" |
platform_cli/helpers.py
Outdated
| result = stdout_call(f"git diff --name-only {base_branch}...HEAD", cwd=cwd, abort=False) | ||
| if not result.strip(): | ||
| return [] | ||
|
|
||
| workspace = Path(cwd) if cwd else Path.cwd() | ||
| return [ | ||
| (workspace / line.strip()).resolve() | ||
| for line in result.strip().splitlines() | ||
| if line.strip() | ||
| ] | ||
| except Exception: | ||
| return [] |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Surface Git failures instead of pretending nothing changed
Catching every Exception here and returning [] means a bad git diff (missing base branch, detached HEAD, no git binary, etc.) quietly pretends there are no changes. Because the caller treats an empty list as “nothing to test”, platform ros test --changed … will exit green without running a single test. That’s a serious false-negative. Please drop the blanket catch (or handle specific, known-benign cases) so genuine failures propagate and fail fast.
Apply this diff:
def get_changed_files(base_branch: str = "main", cwd: Optional[Path] = None) -> List[Path]:
"""Get list of files changed between current HEAD and base branch"""
- try:
- result = stdout_call(f"git diff --name-only {base_branch}...HEAD", cwd=cwd, abort=False)
- if not result.strip():
- return []
-
- workspace = Path(cwd) if cwd else Path.cwd()
- return [
- (workspace / line.strip()).resolve()
- for line in result.strip().splitlines()
- if line.strip()
- ]
- except Exception:
- return []
+ result = stdout_call(f"git diff --name-only {base_branch}...HEAD", cwd=cwd, abort=False)
+ if not result.strip():
+ return []
+
+ workspace = Path(cwd) if cwd else Path.cwd()
+ return [
+ (workspace / line.strip()).resolve()
+ for line in result.strip().splitlines()
+ if line.strip()
+ ]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| result = stdout_call(f"git diff --name-only {base_branch}...HEAD", cwd=cwd, abort=False) | |
| if not result.strip(): | |
| return [] | |
| workspace = Path(cwd) if cwd else Path.cwd() | |
| return [ | |
| (workspace / line.strip()).resolve() | |
| for line in result.strip().splitlines() | |
| if line.strip() | |
| ] | |
| except Exception: | |
| return [] | |
| def get_changed_files(base_branch: str = "main", cwd: Optional[Path] = None) -> List[Path]: | |
| """Get list of files changed between current HEAD and base branch""" | |
| - try: | |
| - result = stdout_call(f"git diff --name-only {base_branch}...HEAD", cwd=cwd, abort=False) | |
| - if not result.strip(): | |
| - return [] | |
| - | |
| - workspace = Path(cwd) if cwd else Path.cwd() | |
| - return [ | |
| - (workspace / line.strip()).resolve() | |
| - for line in result.strip().splitlines() | |
| - if line.strip() | |
| - ] | |
| - except Exception: | |
| result = stdout_call(f"git diff --name-only {base_branch}...HEAD", cwd=cwd, abort=False) | |
| if not result.strip(): | |
| return [] | |
| workspace = Path(cwd) if cwd else Path.cwd() | |
| return [ | |
| (workspace / line.strip()).resolve() | |
| for line in result.strip().splitlines() | |
| if line.strip() | |
| ] |
🧰 Tools
🪛 Ruff (0.13.1)
27-27: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
In platform_cli/helpers.py around lines 17 to 28, the current blanket except
Exception quietly swallows any git failures and returns an empty list; remove
the broad try/except so genuine git errors propagate and fail-fast, or replace
it with targeted exception handling: catch only known-benign cases (e.g.,
subprocess.CalledProcessError when git returns no output you expect, or
FileNotFoundError if git binary missing) and re-raise all other exceptions;
ensure that only legitimate "no changes" is represented by an empty result from
stdout_call and do not convert arbitrary failures into [].
platform_cli/helpers.py
Outdated
|
|
||
| try: | ||
| all_packages = find_packages_with_colcon(workspace_path) | ||
| if not all_packages: | ||
| return [] | ||
|
|
||
| affected_packages = set() | ||
| for changed_file in changed_files: | ||
| for package_name, package_path in all_packages.items(): | ||
| try: | ||
| changed_file.resolve().relative_to(package_path.resolve()) | ||
| affected_packages.add(package_name) | ||
| break | ||
| except ValueError: | ||
| continue | ||
|
|
||
| return list(affected_packages) | ||
| except Exception: | ||
| return [] |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Don’t swallow colcon failures either
Same story here: if find_packages_with_colcon explodes (common when colcon isn’t installed or the workspace is misconfigured) we silently claim there are no affected packages and the test command short-circuits. That masks real setup problems and again yields a false success. Please let the real exception surface (or at least re-raise after logging) so the caller can abort noisily.
Apply this diff:
if not changed_files:
return []
- try:
- all_packages = find_packages_with_colcon(workspace_path)
- if not all_packages:
- return []
+ all_packages = find_packages_with_colcon(workspace_path)
+ if not all_packages:
+ return []
- affected_packages = set()
- for changed_file in changed_files:
- for package_name, package_path in all_packages.items():
- try:
- changed_file.resolve().relative_to(package_path.resolve())
- affected_packages.add(package_name)
- break
- except ValueError:
- continue
-
- return list(affected_packages)
- except Exception:
- return []
+ affected_packages = set()
+ for changed_file in changed_files:
+ for package_name, package_path in all_packages.items():
+ try:
+ changed_file.resolve().relative_to(package_path.resolve())
+ affected_packages.add(package_name)
+ break
+ except ValueError:
+ continue
+
+ return list(affected_packages)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try: | |
| all_packages = find_packages_with_colcon(workspace_path) | |
| if not all_packages: | |
| return [] | |
| affected_packages = set() | |
| for changed_file in changed_files: | |
| for package_name, package_path in all_packages.items(): | |
| try: | |
| changed_file.resolve().relative_to(package_path.resolve()) | |
| affected_packages.add(package_name) | |
| break | |
| except ValueError: | |
| continue | |
| return list(affected_packages) | |
| except Exception: | |
| return [] | |
| if not changed_files: | |
| return [] | |
| all_packages = find_packages_with_colcon(workspace_path) | |
| if not all_packages: | |
| return [] | |
| affected_packages = set() | |
| for changed_file in changed_files: | |
| for package_name, package_path in all_packages.items(): | |
| try: | |
| changed_file.resolve().relative_to(package_path.resolve()) | |
| affected_packages.add(package_name) | |
| break | |
| except ValueError: | |
| continue | |
| return list(affected_packages) |
🧰 Tools
🪛 Ruff (0.13.1)
54-54: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
In platform_cli/helpers.py around lines 37 to 55, the broad try/except currently
swallows any exception from find_packages_with_colcon (e.g. when colcon is
missing) and returns an empty list; change this so failures surface: remove the
outer blanket except that returns [] (or replace it with a limited except that
logs the error and then re-raises), allowing the original exception to propagate
to the caller so the process can abort noisily; keep the inner logic that
computes affected_packages and the early return when no packages are found.
MrBlenny
force-pushed
the
ros-test-changed
branch
from
111ef2f to
43660d1
Compare
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
MrBlenny
changed the title
--changed flag to platform ros test
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
platform_cli/groups/ros.py (3)
86-86: Align Click types with annotations (tuple vs list).Click with
multiple=Trueandnargs=-1returns tuples, but the function types useList[str]. PreferSequence[str]orTuple[str, ...]to match runtime and improve typing. As per coding guidelines.Apply this diff to the signature:
- def test( - package: List[str], + def test( + package: Tuple[str, ...], results_dir: Path, watch: bool, build: bool, retest_until_pass: int, include_dependents: bool, - args: List[str], + args: Tuple[str, ...], ):And add the import (outside this hunk):
from typing import List, Tuple # replace existing importAlso applies to: 113-114, 119-119
166-169: Results collection skips the “all packages” case.When no packages were selected, nothing is copied.
collect_xunit_xmlsalready treats a falsy package as “*”, so invoke it once in that case.Apply this diff:
- if results_dir and results_dir.exists(): - for pkg in packages_to_test: - collect_xunit_xmls(results_dir, pkg) + if results_dir and results_dir.exists(): + if packages_to_test: + for pkg in packages_to_test: + collect_xunit_xmls(results_dir, pkg) + else: + collect_xunit_xmls(results_dir, "")
126-129: Minor: avoid buildingpackages_strwhen the list is empty.Tiny nit to reduce work and keep variables tightly scoped.
Apply this diff:
- packages_to_test = list(package) if package else [] - packages_str = " ".join(packages_to_test) + packages_to_test = list(package) if package else []
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
platform_cli/groups/ros.py(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
⚙️ CodeRabbit configuration file
**/*.py: - Use Python 3.8+ features (f-strings, walrus operator, positional-only parameters)
- Use dependency injection patterns - inject dependencies through constructors rather than creating them internally
- Inject configuration and external services (databases, APIs, ROS nodes) rather than hardcoding them
- Design classes with single responsibility principle - each class should have one clear purpose
- Include comprehensive type annotations using typing module (List, Dict, Optional, Union, etc.)
- Prefer built-in functions like
next(),any(),all(),enumerate()over manual loops- Follow ROS2 best practices: use lifecycle nodes, quality of service (QoS) profiles, and parameter declarations
- Design functions and classes to be easily unit testable - avoid tight coupling to ROS nodes
- Use dependency injection patterns to make ROS dependencies mockable in tests
- Implement proper error handling with specific exception types, avoid bare
except:- Use dataclasses or Pydantic models for structured data instead of dictionaries
- Leverage ROS2 latest features: composition, lifecycle management, parameter callbacks
- Add unit tests for complex business logic using pytest, mock ROS dependencies
- Use context managers (
withstatements) for resource management- Prefer list/dict comprehensions and generator expressions over loops where readable
- Follow PEP 8 naming conventions and use meaningful variable names
- Use logging instead of print statements, configure ROS2 logging levels appropriately
- Implement proper cleanup in node destructors and signal handlers
- Use async/await for non-blocking operations where appropriate
- Structure code with clear separation between ROS communication and business logic
- Use piknik's generate_parameter_library for declarative parameters which make unit testing logic that uses parameters easier
- Ensure changes to ROS params, services, publishers and subscribers are documented in the design.md file
Files:
platform_cli/groups/ros.py
| @click.option( | ||
| "--include-dependents", | ||
| type=bool, | ||
| is_flag=True, | ||
| default=False, | ||
| help="Also test packages that depend on changed packages (used with --changed)", | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
Dependents vs dependencies: --packages-up-to is the wrong selector for “dependents”.
--packages-up-to selects the package(s) and their dependencies. To include packages that depend on the selected ones, use --packages-above or --packages-above-and-dependencies. Current code will test dependencies, not dependents. This is a logic bug that will miss intended targets.
Apply this diff:
- @click.option(
- "--include-dependents",
+ @click.option(
+ "--include-dependents",
type=bool,
is_flag=True,
default=False,
- help="Also test packages that depend on changed packages (used with --changed)",
+ help="Also test packages that depend on the selected packages",
)- if packages_to_test:
- if include_dependents:
- args_str_test += f" --packages-up-to {packages_str}"
- else:
- args_str_test += f" --packages-select {packages_str}"
+ if packages_to_test:
+ if include_dependents:
+ args_str_test += f" --packages-above-and-dependencies {packages_str}"
+ else:
+ args_str_test += f" --packages-select {packages_str}"Please confirm the intended semantics and that --changed is implemented elsewhere (your help text references it but it’s not defined here). Run:
Also verify colcon selectors:
Also applies to: 130-134
🏁 Script executed:
#!/bin/bash
# Check for --changed usage in the repo
rg -nP --type=python -C2 '\-\-changed'Length of output: 30
🌐 Web query:
In colcon, what do --packages-select, --packages-up-to, --packages-above, and --packages-above-and-dependencies select?
💡 Result:
-
--packages-select <pkg...>: selects only the named package(s). [1]
-
--packages-up-to <pkg...>: selects the named package(s) and all their recursive dependencies (i.e., packages they depend on). [1]
-
--packages-above <pkg...>: selects the named package(s) and all packages that (recursively) depend on them (i.e., their dependents). [1]
-
--packages-above-and-dependencies <pkg...>: selects the named package(s), all packages that depend on them, and those dependents' recursive dependencies. (Equivalent to union of --packages-above and their dependencies.) [1]
Reference: colcon package selection arguments. [1] https://colcon.readthedocs.io/en/main/reference/package-selection-arguments.html
🏁 Script executed:
#!/bin/bash
# Show context around the click.option snippet in ros.py
fd ros.py -x sed -n '1,200p' {}
# Search for --changed flag across all Python files
rg -g "*.py" -n "\-\-changed" -C2Length of output: 7827
Fix dependents logic in ros test
The --include-dependents flag currently:
- Uses
--packages-up-to, which selects dependencies rather than dependents. - References a non-existent
--changedoption in its help text.
Apply:
- Change help to
help="Also test packages that depend on the selected packages". - In the test command’s selector logic, replace
--packages-up-towith--packages-above-and-dependencieswheninclude_dependentsis true.
🤖 Prompt for AI Agents
In platform_cli/groups/ros.py around lines 104 to 110, update the option help
and the selector logic: change the help string for --include-dependents to "Also
test packages that depend on the selected packages" (removing the incorrect
reference to --changed), and in the test command selector code branch where
include_dependents is True, replace use of the --packages-up-to selector flag
with --packages-above-and-dependencies so the command selects dependents (above)
and their dependencies instead of only dependencies.
| args_str_build = args_str | ||
| if packages_to_test: | ||
| for pkg in packages_to_test: | ||
| args_str_build += f" --package {pkg}" | ||
| call(" ".join(["platform ros build", args_str_build])) |
There was a problem hiding this comment.
Build step only builds the last package; construct a single colcon build with proper selection.
Passing --package repeatedly to platform ros build (which accepts a single --package) means Click will take the last value. Also, when include_dependents is set we should mirror the same selection here.
Apply this diff to build in-place (avoids re-invoking our CLI and ensures correct selection):
- if build:
- args_str_build = args_str
- if packages_to_test:
- for pkg in packages_to_test:
- args_str_build += f" --package {pkg}"
- call(" ".join(["platform ros build", args_str_build]))
+ if build:
+ args_str_build = args_str
+ if packages_to_test:
+ packages_str = " ".join(packages_to_test)
+ if include_dependents:
+ args_str_build += f" --packages-above-and-dependencies {packages_str}"
+ else:
+ args_str_build += f" --packages-select {packages_str}"
+ call(
+ " ".join(
+ [
+ "colcon build",
+ "--merge-install",
+ "--symlink-install",
+ f"--install-base /opt/greenroom/{env['PLATFORM_MODULE']}",
+ args_str_build,
+ ]
+ )
+ )Alternatively, update platform ros build to accept multiple=True on --package and apply a single --packages-select with the joined list.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| args_str_build = args_str | |
| if packages_to_test: | |
| for pkg in packages_to_test: | |
| args_str_build += f" --package {pkg}" | |
| call(" ".join(["platform ros build", args_str_build])) | |
| if build: | |
| args_str_build = args_str | |
| if packages_to_test: | |
| packages_str = " ".join(packages_to_test) | |
| if include_dependents: | |
| args_str_build += f" --packages-above-and-dependencies {packages_str}" | |
| else: | |
| args_str_build += f" --packages-select {packages_str}" | |
| call( | |
| " ".join( | |
| [ | |
| "colcon build", | |
| "--merge-install", | |
| "--symlink-install", | |
| f"--install-base /opt/greenroom/{env['PLATFORM_MODULE']}", | |
| args_str_build, | |
| ] | |
| ) | |
| ) |
🤖 Prompt for AI Agents
In platform_cli/groups/ros.py around lines 138-142, the current loop appends
multiple "--package" flags which Click accepts only once so only the last
package is built; replace this by constructing a single in-place colcon build
invocation that uses "--packages-select" with a comma/space-joined list of
packages (and include dependents when include_dependents is set), or
alternatively change the platform ros build command to accept multiple=True for
the "--package" option and internally join them into one "--packages-select"
argument; ensure you call the colcon build directly (build-in-place) rather than
re-invoking the CLI so all selected packages are built correctly.
Summary by CodeRabbit