Skip to content

ci: remove nlohmann_json workaround now that unh_marine_autonomy#228 landed (#76)#79

Merged
rolker merged 7 commits into
jazzyfrom
feature/issue-76
Jun 27, 2026
Merged

ci: remove nlohmann_json workaround now that unh_marine_autonomy#228 landed (#76)#79
rolker merged 7 commits into
jazzyfrom
feature/issue-76

Conversation

@rolker

@rolker rolker commented Jun 27, 2026

Copy link
Copy Markdown
Owner

Closes #76.

What

Removes the nlohmann_json CI workaround now that
rolker/unh_marine_autonomy#228
(merged jazzy 2026-06-27) fixed the invalid rosdep key. cube clones
unh_marine_autonomy@jazzy, so the bad key is gone.

  • .github/workflows/ci.yml — drop nlohmann_json from ROSDEP_SKIP_KEYS
    (the CI-time rosdep resolve no longer hits it).
  • .github/ci/Dockerfile — drop the explicit nlohmann-json3-dev install;
    the dynamic rosdep check auto-skip now resolves nlohmann-json-dev on its
    own, so the dep installs via the normal rosdep path. The Dockerfile change
    triggers an ci-image.yml image rebuild on merge.

Option B (per #76 plan-review checkpoint)

Kept marine_nav_interfaces / marine_nav_tasks in ROSDEP_SKIP_KEYS and
the COLCON_IGNORE hook as intentional pruning of mission_manager* /
integration-tests / backscatter — packages outside cube's build graph. Building
them in cube CI has no benefit to cube correctness and wastes time. Comments
reframed from "#228-pending workaround" to intentional scoping. Option A
(clone unh_marine_navigation + full-monorepo build) was considered and
rejected.

Verification

  • Local --no-cache image build against the fixed unh_marine_autonomy@jazzy:
    baked skip set is now marine_nav_interfaces marine_nav_tasks only
    (nlohmann_json gone — resolves naturally), nlohmann-json3-dev installed via
    the resolved key, build green.
  • Pre-push review-code: approved, 0 must-fix (2 comment-accuracy suggestions
    applied).

CI-only change; no code, behavior, or test changes.


Authored-By: Claude Code Agent
Model: Claude Opus 4.8

Claude Code Agent added 7 commits June 27, 2026 10:43
Drop nlohmann_json workaround (Option B: keep marine_nav pruning as intentional scope)
unh_marine_autonomy#228 (merged jazzy 2026-06-27) fixed the nlohmann_json
rosdep key (-> nlohmann-json-dev) and added a root dependencies.repos. cube
clones unh_marine_autonomy@jazzy, so the bad key is gone:

- ci.yml: drop nlohmann_json from ROSDEP_SKIP_KEYS (the CI-time rosdep resolve
  no longer hits it).
- Dockerfile: drop the explicit nlohmann-json3-dev install — the dynamic
  rosdep-check auto-skip now resolves nlohmann-json-dev on its own, so the dep
  installs via the normal rosdep path.

Option B (per #76): KEEP the marine_nav_* skip-keys + COLCON_IGNORE as
intentional pruning of mission_manager*/integration-tests/backscatter, which are
outside cube's build graph. Comments reframed from '#228-pending workaround' to
'intentional scoping'. Option A (clone unh_marine_navigation + full-monorepo
build) was rejected: no benefit to cube correctness.
Address two review suggestions on the ci.yml prune comment:
- Separate marine_mbes_backscatter_store (no marine_nav dep — COLCON_IGNORE'd
  purely for scope) from mission_manager*/integration-tests (which declare
  marine_nav_* and thus need the skip-key).
- Stop implying marine_nav was an upstream bug fixed by #228; only the invalid
  nlohmann_json rosdep key was — marine_nav skipping is cube's deliberate
  pruning choice, not a resolved gap.
Copilot AI review requested due to automatic review settings June 27, 2026 16:41

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@rolker
rolker merged commit 07260a2 into jazzy Jun 27, 2026
1 of 2 checks passed
@rolker
rolker deleted the feature/issue-76 branch June 27, 2026 16:43
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.

CI: remove nlohmann_json workaround (unh_marine_autonomy#228 landed); decide marine_nav pruning

2 participants