Skip to content

Allow building an upgrade from an async function#1383

Merged
tomaka merged 4 commits into
libp2p:masterfrom
tomaka:upgrade-from-fn
Jan 15, 2020
Merged

Allow building an upgrade from an async function#1383
tomaka merged 4 commits into
libp2p:masterfrom
tomaka:upgrade-from-fn

Conversation

@tomaka

@tomaka tomaka commented Jan 9, 2020

Copy link
Copy Markdown
Member

This is a quality-of-life change, with the objective of trying to make rust-libp2p easier to use.

I think it's quite nice and comprehensible to be able to write upgrades as shown in the doc-comment example.

Comment thread core/src/upgrade/from_fn.rs Outdated
/// return Err(upgrade::ReadOneError::from(io::Error::from(io::ErrorKind::Other)));
/// }
/// }
///

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
///

Comment thread core/src/upgrade/from_fn.rs Outdated
{
type Output = Out;
type Error = Err;
type Future = Pin<Box<Fut>>; // TODO: don't box the future

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do you want to file an issue for the TODOs in here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We could also do #1384 first, I'm not sure.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Will rebase on top of #1384 once it's merged.

@tomaka

tomaka commented Jan 14, 2020

Copy link
Copy Markdown
Member Author

Updated and concerns addressed!

@tomaka

tomaka commented Jan 15, 2020

Copy link
Copy Markdown
Member Author

Any objection to merging?

@twittner

Copy link
Copy Markdown
Contributor

Not at all.

@tomaka tomaka merged commit 89acb0d into libp2p:master Jan 15, 2020
@tomaka tomaka deleted the upgrade-from-fn branch January 15, 2020 13:57
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