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

Make the structs in nodes generic over PeerId #881

Merged
merged 2 commits into from
Jan 23, 2019

Conversation

tomaka
Copy link
Member

@tomaka tomaka commented Jan 22, 2019

Instead of hard-coding the ID of a peer as PeerId, make it generic.
This is a small breaking change as we implement AsRef<[u8]> on PeerId, which can cause an inference error, but we can consider this as acceptable.

// Set the state of the task to `Connected`.
let former_task_id = self.parent.nodes.insert(self.peer_id.clone(), self.id);
let _former_state = self.parent.tasks.insert(self.id, TaskState::Connected(self.peer_id.clone()));
debug_assert_eq!(_former_state, Some(TaskState::Pending));
debug_assert!(_former_state == Some(TaskState::Pending));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the change from debug_assert_eq to debug_assert here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise we have to require Debug on the TPeerId.

Copy link

@senden9 senden9 Jan 22, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this not required here? https://github.com/libp2p/rust-libp2p/pull/881/files#diff-87276c9dd75246456f7330545b24dd12R128

       TPeerId: Eq + Hash + Clone + fmt::Debug,

Edit: Oh, I understand. Impl <…> fmt::Debug for … TPeerId: … + fmt::Debug

Copy link
Contributor

@twittner twittner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎆

@tomaka tomaka merged commit d9b30c6 into libp2p:master Jan 23, 2019
@tomaka tomaka deleted the peer-id-generic branch January 23, 2019 16:44
@ghost ghost removed the in progress label Jan 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants