Skip to content

Dead code removal. - #297

Open
mosfet80 wants to merge 3 commits into
PickNikRobotics:ros2from
mosfet80:fixCompile
Open

mosfet80 wants to merge 3 commits into
PickNikRobotics:ros2from
mosfet80:fixCompile

Conversation

@mosfet80

Copy link
Copy Markdown
Contributor

Clean code
fix compile

nbbrooks added a commit that referenced this pull request Aug 2, 2026
Three problems, all of which make CI on `ros2` uninformative today.

1. fail-fast

The matrix had no `fail-fast: false`, so it defaulted to true. The rolling
job fails in ~60s (problem 2) and GitHub then cancels every sibling before
it can report. From PR #300, run 30764760558 attempt 3:

  rolling-main + ccov  20:49:41 -> 20:50:45  failure
  humble-main          20:49:47 -> 20:50:51  cancelled
  humble-testing       20:49:47 -> 20:50:52  cancelled
  rolling-testing      20:49:41 -> 20:50:52  cancelled

The matrix reported nothing at all, and re-running reproduced it exactly.

2. rolling + ROS_REPO: main on Resolute

Rolling's base OS moved to Ubuntu Resolute. Rolling's `main` apt repo has no
Resolute packages yet, so the job dies before CMake:

  'sudo apt-get install ... ros-rolling-ros-environment' returned with 100
  'setup_rosdep' returned with code '100' after 0 min 7 sec

Not PR-specific: PR #297 fails the identical job. Switch it to `testing`,
which does work on Resolute, and pin OS_CODE_NAME explicitly rather than
relying on industrial_ci's default -- that default has changed before.

Mark it non-blocking, matching the policy moveit2 applies to its own
rolling-resolute job ("non-blocking until all Resolute packages are
released"). CCOV rides on this job, so coverage is non-blocking too.

3. Three released distros had no CI at all

This repo ships a single `ros2` branch to five distros but only tested two.
jazzy (4.1.4-4), kilted (4.1.4-4) and lyrical (4.1.4-5) are all released
from this branch with zero coverage. Unlike moveit2 -- which can omit
distros from `main` because it maintains per-distro branches -- there is no
second branch here to pick up the slack.

That gap lands exactly on the case that matters most for the Qt work:
jazzy and kilted are Ubuntu Noble, the platform where Qt5 and Qt6 can be
installed side by side, which is precisely what the rviz-version Qt gate in
CMakeLists guards against. humble covers jammy/Qt5 and rolling covers
resolute/Qt6; the interesting middle was untested.

lyrical is Resolute and released, so its `main` repo is populated -- it
gives a blocking Qt6-on-Resolute job that should stay green, instead of
leaving all Qt6 coverage on the non-blocking rolling job.

Restore the rolling+main entry once Rolling publishes to `main` for
Resolute, and drop the NONBLOCKING flag at the same time.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
nbbrooks added a commit that referenced this pull request Aug 2, 2026
Three problems, all of which make CI on `ros2` uninformative today.

1. fail-fast

The matrix had no `fail-fast: false`, so it defaulted to true. The rolling
job fails in ~60s (problem 2) and GitHub then cancels every sibling before
it can report. From PR #300, run 30764760558 attempt 3:

  rolling-main + ccov  20:49:41 -> 20:50:45  failure
  humble-main          20:49:47 -> 20:50:51  cancelled
  humble-testing       20:49:47 -> 20:50:52  cancelled
  rolling-testing      20:49:41 -> 20:50:52  cancelled

The matrix reported nothing at all, and re-running reproduced it exactly.

2. rolling + ROS_REPO: main on Resolute

Rolling's base OS moved to Ubuntu Resolute. Rolling's `main` apt repo has no
Resolute packages yet, so the job dies before CMake:

  'sudo apt-get install ... ros-rolling-ros-environment' returned with 100
  'setup_rosdep' returned with code '100' after 0 min 7 sec

Not PR-specific: PR #297 fails the identical job. Switch it to `testing`,
which does work on Resolute, and pin OS_CODE_NAME explicitly rather than
relying on industrial_ci's default -- that default has changed before.

Mark it non-blocking, matching the policy moveit2 applies to its own
rolling-resolute job ("non-blocking until all Resolute packages are
released"). CCOV rides on this job, so coverage is non-blocking too.

3. Three released distros had no CI at all

This repo ships a single `ros2` branch to five distros but only tested two.
jazzy (4.1.4-4), kilted (4.1.4-4) and lyrical (4.1.4-5) are all released
from this branch with zero coverage. Unlike moveit2 -- which can omit
distros from `main` because it maintains per-distro branches -- there is no
second branch here to pick up the slack.

That gap lands exactly on the case that matters most for the Qt work:
jazzy and kilted are Ubuntu Noble, the platform where Qt5 and Qt6 can be
installed side by side, which is precisely what the rviz-version Qt gate in
CMakeLists guards against. humble covers jammy/Qt5 and rolling covers
resolute/Qt6; the interesting middle was untested.

lyrical is Resolute and released, so its `main` repo is populated -- it
gives a blocking Qt6-on-Resolute job that should stay green, instead of
leaving all Qt6 coverage on the non-blocking rolling job.

Restore the rolling+main entry once Rolling publishes to `main` for
Resolute, and drop the NONBLOCKING flag at the same time.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 13.41%. Comparing base (101b1d1) to head (f3abc5c).
⚠️ Report is 12 commits behind head on ros2.

Files with missing lines Patch % Lines
src/tf_visual_tools.cpp 0.00% 1 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@            Coverage Diff             @@
##             ros2     #297      +/-   ##
==========================================
- Coverage   19.86%   13.41%   -6.45%     
==========================================
  Files           5       12       +7     
  Lines        1858     2319     +461     
  Branches        0      131     +131     
==========================================
- Hits          369      311      -58     
- Misses       1489     1998     +509     
- Partials        0       10      +10     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@nbbrooks

nbbrooks commented Aug 3, 2026

Copy link
Copy Markdown
Member

@mosfet80 - the dead code removal is a nice improvement. I think the other items have been resolved by other means. Could you update this PR's scope to the dead code removal?

@mosfet80 mosfet80 changed the title Fix compile Dead code removal. Aug 13, 2026
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.

3 participants