-
Notifications
You must be signed in to change notification settings - Fork 90
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
cmd/bootnode: move bootnode to own package #1512
Conversation
Codecov ReportBase: 54.41% // Head: 54.44% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #1512 +/- ##
==========================================
+ Coverage 54.41% 54.44% +0.03%
==========================================
Files 149 150 +1
Lines 19058 19058
==========================================
+ Hits 10370 10376 +6
+ Misses 7292 7284 -8
- Partials 1396 1398 +2
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
HTTPAddr string | ||
P2PConfig p2p.Config | ||
LogConfig log.Config | ||
AutoP2PKey bool |
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.
why the auto
prefix?
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.
this is existing logic, I'm just moving it.
RelayLogLevel string | ||
} | ||
|
||
// Run starts an Obol libp2p-tcp-relay and udp-discv5 bootnode. |
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.
// Run starts an Obol libp2p-tcp-relay and udp-discv5 bootnode. | |
// Run starts a libp2p-tcp-relay and udp-discv5 bootnode. |
Since anyone can start a bootnode, no?
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.
yeah but it is a "obol bootnode"
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.
or a "charon bootnode" ?
) | ||
|
||
<-ctx.Done() | ||
_ = tcpNode.Close() |
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.
why do we not handle error here?
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.
this is existing logic, I'm just moving it.
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.
LGTM!
Co-authored-by: Abhishek Kumar <[email protected]>
Move bootnode into its own package in preparation for adding metrics and more stuff to it.
category: refactor
ticket: #1506