Skip to content

Conversation

@tomaka
Copy link
Member

@tomaka tomaka commented Dec 4, 2017

No description provided.

@tomaka tomaka force-pushed the fix-err-transport branch from df5585e to 7aee015 Compare December 5, 2017 09:22
@tomaka
Copy link
Member Author

tomaka commented Dec 5, 2017

Rebased over master ; travis tests won't pass because of #61

@tomaka tomaka force-pushed the fix-err-transport branch from 7aee015 to ff2fe98 Compare December 6, 2017 11:02
let negotiated = multistream_select::listener_select_proto(connection, iter);
negotiated.map(move |(upgrade_id, conn)| (upgrade_id, conn, upgrade, client_addr))
.map_err(|_| panic!()) // TODO:
.map_err(|err| IoError::new(IoErrorKind::Other, err))
Copy link
Contributor

Choose a reason for hiding this comment

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

Style nit: I prefer use std::io; to use std::io::Error as IoError;, but that could be argued as personal taste.

}

fn cause(&self) -> Option<&error::Error> {
match *self {
Copy link
Contributor

Choose a reason for hiding this comment

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

Style nit: maybe use an if let?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nit: A match is better if we add more variants in the future


fn cause(&self) -> Option<&error::Error> {
match *self {
MultistreamSelectError::IoError(ref err) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we repeat Error/MultistreamSelect twice in this? Can it be enum Error { Io(...), ... }? We know we're in the multistream select crate already.

@tomaka tomaka force-pushed the fix-err-transport branch from ff2fe98 to 809ff7e Compare December 7, 2017 15:22
@tomaka tomaka merged commit d07a83b into libp2p:master Dec 7, 2017
@tomaka tomaka deleted the fix-err-transport branch December 7, 2017 15:42
mxinden pushed a commit to mxinden/rust-libp2p that referenced this pull request Nov 20, 2020
dkuehr pushed a commit to openmina/rust-libp2p that referenced this pull request Oct 24, 2023
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.

2 participants