Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NetUtils: simplify logic in determineInterfaces #257

Merged
merged 6 commits into from
Oct 18, 2021

Conversation

scpeters
Copy link
Member

🦟 Bug fix

Removes an unused variable and simplifies complex logic in determineInterfaces method.

Summary

  • 2f6056b: The preferred_ip array is initialized to {0} and no code writes to it; it is only read from, so just remove it.
  • 61ce68f: Now there is an if/else if/else block that runs the same code in each branch, so just remove the logic and run that code unconditionally.

I believe this is functionally equivalent to the current code.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • 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

The preferred_ip array is initialized to {0} and no code
writes to it; it is only read from, so just remove it.

Signed-off-by: Steve Peters <[email protected]>
Each branch executes the same code, so just run that code
unconditionally.

Signed-off-by: Steve Peters <[email protected]>
@scpeters scpeters requested a review from caguero August 23, 2021 17:50
@github-actions github-actions bot added the 🏢 edifice Ignition Edifice label Aug 23, 2021
The poll() API with integer timeout is deprecated.
Use std::chrono::duration instead.

Signed-off-by: Steve Peters <[email protected]>
@codecov
Copy link

codecov bot commented Sep 29, 2021

Codecov Report

Merging #257 (12f2211) into ign-transport10 (95bb75a) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@                 Coverage Diff                 @@
##           ign-transport10     #257      +/-   ##
===================================================
+ Coverage            89.08%   89.10%   +0.02%     
===================================================
  Files                   51       51              
  Lines                 4772     4765       -7     
===================================================
- Hits                  4251     4246       -5     
+ Misses                 521      519       -2     
Impacted Files Coverage Δ
src/NetUtils.cc 72.81% <100.00%> (+0.08%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 95bb75a...12f2211. Read the comment docs.

@scpeters scpeters merged commit 347ff00 into gazebosim:ign-transport10 Oct 18, 2021
@scpeters scpeters deleted the unused_preferred_ip branch October 18, 2021 18:56
@osrf-triage
Copy link

This pull request has been mentioned on Gazebo Community. There might be relevant details there:

https://community.gazebosim.org/t/new-ignition-releases-2022-03-25-fortress-edifice-citadel/1343/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏢 edifice Ignition Edifice
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants