Skip to content

Use tl_expected from libexpected-dev instead#572

Merged
christophfroehlich merged 4 commits intomasterfrom
tl-expected
Mar 24, 2026
Merged

Use tl_expected from libexpected-dev instead#572
christophfroehlich merged 4 commits intomasterfrom
tl-expected

Conversation

@christophfroehlich
Copy link
Copy Markdown
Member

@fmauch
Copy link
Copy Markdown

fmauch commented Mar 23, 2026

@christophfroehlich Since PickNikRobotics/generate_parameter_library#322 is merged (and released) I guess this can be undrafted and reviewed by now?

@christophfroehlich christophfroehlich marked this pull request as ready for review March 23, 2026 13:24
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Mar 23, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.85%. Comparing base (f2eb331) to head (2bc4e0d).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #572   +/-   ##
=======================================
  Coverage   83.85%   83.85%           
=======================================
  Files          30       30           
  Lines        2094     2094           
  Branches      114      114           
=======================================
  Hits         1756     1756           
  Misses        268      268           
  Partials       70       70           
Flag Coverage Δ
unittests 83.85% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

Copy link
Copy Markdown

@fmauch fmauch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent of the actual change, but: it seems unconventional that this header uses headers from rsl and tl without specifying those in the package.xml.

I am aware that those are transitive dependencies coming from GPL, which is indeed a dependency, but IMHO it would be cleaner to explicitly declare the dependency here, as well, if the headers are included.

@christophfroehlich
Copy link
Copy Markdown
Member Author

Independent of the actual change, but: it seems unconventional that this header uses headers from rsl and tl without specifying those in the package.xml.

I am aware that those are transitive dependencies coming from GPL, which is indeed a dependency, but IMHO it would be cleaner to explicitly declare the dependency here, as well, if the headers are included.

you are right, I just added them. maybe this repo is not the best place for this header file, but we haven't found a better place back then

@christophfroehlich
Copy link
Copy Markdown
Member Author

maybe I should even push this to the upstream repo as we maintain it now also

Copy link
Copy Markdown

@fmauch fmauch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think, it's fair to leave the header here, since it's used here. Moving it to the GPL repo might be fine, as well.

@christophfroehlich christophfroehlich merged commit 88f5298 into master Mar 24, 2026
16 checks passed
@christophfroehlich christophfroehlich deleted the tl-expected branch March 24, 2026 18:30
@christophfroehlich christophfroehlich added backport-humble Triggers PR backport to ROS 2 humble. backport-jazzy Triggers PR backport to ROS 2 jazzy. backport-kilted Triggers PR backport to ROS 2 kilted. labels Mar 26, 2026
mergify bot pushed a commit that referenced this pull request Mar 26, 2026
(cherry picked from commit 88f5298)

# Conflicts:
#	.github/workflows/rolling-semi-binary-build-win.yml
#	package.xml
mergify bot pushed a commit that referenced this pull request Mar 26, 2026
(cherry picked from commit 88f5298)

# Conflicts:
#	.github/workflows/rolling-semi-binary-build-win.yml
mergify bot pushed a commit that referenced this pull request Mar 26, 2026
(cherry picked from commit 88f5298)

# Conflicts:
#	.github/workflows/rolling-semi-binary-build-win.yml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-humble Triggers PR backport to ROS 2 humble. backport-jazzy Triggers PR backport to ROS 2 jazzy. backport-kilted Triggers PR backport to ROS 2 kilted.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants