gazebosim / gz-transport

Transport library for component communication based on publication/subscription and service calls.

https://gazebosim.org

Apache License 2.0

29 stars 42 forks source link

Remove deprecations in Harmonic #426

Closed caguero closed 1 year ago

caguero commented 1 year ago

🎉 Maintenance

Remove deprecations

Checklist

[x] Signed all commits for DCO
[ ] Added tests
[ ] Added example and/or tutorial
[ ] Updated documentation (as needed)
[ ] Updated migration guide (as needed)
[ ] Consider updating Python bindings (if the library has them)
[ ] codecheck passed (See contributing)
[ ] All tests passed (See test coverage)
[ ] While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

codecov[bot] commented 1 year ago

Codecov Report

Merging #426 (fbd8a01) into main (3a11524) will increase coverage by 0.35%. The diff coverage is 100.00%.

:exclamation: Current head fbd8a01 differs from pull request most recent head e5d3ef1. Consider uploading reports for the commit e5d3ef1 to get more accurate results

@@            Coverage Diff             @@
##             main     #426      +/-   ##
==========================================
+ Coverage   87.40%   87.75%   +0.35%     
==========================================
  Files          59       59              
  Lines        5747     5693      -54     
==========================================
- Hits         5023     4996      -27     
+ Misses        724      697      -27

Files Changed	Coverage Δ
include/gz/transport/Discovery.hh	`86.95% <ø> (+0.94%)`	:arrow_up:
log/src/Log.cc	`79.45% <ø> (+0.73%)`	:arrow_up:
src/NetUtils.cc	`75.22% <ø> (+1.12%)`	:arrow_up:
src/NodeOptions.cc	`100.00% <ø> (+3.70%)`	:arrow_up:
src/NodeShared.cc	`78.99% <100.00%> (+0.95%)`	:arrow_up:

... and 1 file with indirect coverage changes

mjcarroll commented 1 year ago

@osrf-jenkins retest this please

caguero commented 1 year ago

@Crola1702 , do you know why the Ubuntu Focal job gets stuck?

Crola1702 commented 1 year ago

I think for harmonic we're not supporting Focal but Jammy, right? Maybe it's triggered as a Jammy job, but checks are expecting a Focal job :thinking:

Crola1702 commented 1 year ago

Maybe @j-rivero has more context on this?

caguero commented 1 year ago

I think for harmonic we're not supporting Focal but Jammy, right? Maybe it's triggered as a Jammy job, but checks are expecting a Focal job thinking

As far as I know it's only Jammy (and the future 24.04). Not sure why it's triggered...

mjcarroll commented 1 year ago

As far as I know it's only Jammy (and the future 24.04). Not sure why it's triggered...

It's because this PR was opened before we disabled the focal job for the target branch. It can safely be ignored.

caguero commented 1 year ago

As far as I know it's only Jammy (and the future 24.04). Not sure why it's triggered...

It's because this PR was opened before we disabled the focal job for the target branch. It can safely be ignored.

I see. I don't have superpowers to merge this PR without all the checks passing...

mjcarroll commented 1 year ago

Is the failing windows test okay?

mjcarroll commented 1 year ago

I see. I don't have superpowers to merge this PR without all the checks passing...

I removed the branch protection check, can you merge it now?