Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Create Networking Service #49
Create Networking Service #49
Changes from 31 commits
25c550b
60ff052
facc4ea
e9c7346
eb24916
0955993
f3c013b
1fe9402
a2e6fd2
f0f2f7a
d90d686
eca7248
24263c0
4076fbb
830ca7e
c20a7fb
96b5f12
21552e7
8435720
0c2585d
ec52afe
ab037a0
723eedb
efd0061
f7f5962
c9b4d32
cc8847f
092d782
3a68f2f
54bfd04
c03f05b
2a73a4e
b0812f1
59c2477
33bdf70
8e29083
18c0844
46166fb
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
I am curious what
#[behaviour(ignore)]
and#[behaviour(out_event = "MyBehaviourEvent", poll_method = "poll")]
do? Can you explain?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.
#[derive(NetworkBehaviour)] imparts the NetworkBehaviour trait to MyBehaviour. Then #[behaviour(out_event = "MyBehaviourEvent", poll_method = "poll")] are the attributes that the macro expects, to give it more context.
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.
I think it would be beneficial to have comments for some of these functions below
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.
These functions are all inherited through a trait. Do you think that it would still need documentation? Also they are not functions that we EVER should call. They are called by Libp2p
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.
function returns a result, are you sure you want this to be a panic?
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. I want it to panic if the user gives an invalid address to listen on.
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.
same here, the function signature is for a result, but there are no error types returned anywhere in the function. If intended to be a panic for these, change the function signature to just return Self
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, you're right. Ill change that to return just Self
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. I want it to panic if the user gives an invalid address to listen on.
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 are you changing the error to an Other io error? Are you sure this is the type of error this should return?
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. We get some sort of libp2p transport error, and i want to convert it to an IO error