-
Notifications
You must be signed in to change notification settings - Fork 6
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
Fix empty address check in v0.5.11 #156
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## release-v0.5.x #156 +/- ##
=================================================
Coverage ? 60.63%
=================================================
Files ? 57
Lines ? 5032
Branches ? 0
=================================================
Hits ? 3051
Misses ? 1652
Partials ? 329 ☔ View full report in Codecov by Sentry. |
5eedcbc
to
cbe7df5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comments
dagsync/ipnisync/sync.go
Outdated
addrs := make([]multiaddr.Multiaddr, 0, len(peerInfo.Addrs)) | ||
for _, addr := range peerInfo.Addrs { | ||
if addr != nil { | ||
addrs = append(addrs, addr) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might as well use cleanPeerAddrInfo
for this.
Also, would it be better to remove nil addrs before the if len(peerInfo.Addrs) == 0
check above? Then the check below would not be necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup thanks Andrew, this PR is being tested and needs cleanup/propagation of changes to the fixes against main
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If nil adds are put in peerstore, would they also be returned by it?
664038f
to
3d0a287
Compare
Check for empty addrs earlier when instantiating syncer or subscriber. Patch fix for v0.5.11. See #155 for fix on `main`.
3d0a287
to
d257209
Compare
Check for empty addrs earlier when instantiating syncer.
Patch fix for v0.5.11.
See #155 for fix on
main
.