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

Feat/protocol prefs #89

Merged
merged 11 commits into from
Aug 23, 2016
Merged

Feat/protocol prefs #89

merged 11 commits into from
Aug 23, 2016

Conversation

whyrusleeping
Copy link
Contributor

This adds support for remembering which of a set of protocols works for a given peer. It also uses protocols learned from the identify handshake to seed this knowledge if available.

@whyrusleeping whyrusleeping added the status/in-progress In progress label Aug 17, 2016
@whyrusleeping whyrusleeping force-pushed the feat/protocol-prefs branch 2 times, most recently from 57fa777 to 0aaf4cf Compare August 18, 2016 04:06
@whyrusleeping
Copy link
Contributor Author

it appears theres actually a problem on osx here

@whyrusleeping
Copy link
Contributor Author

alright, fixed the osx thing, apparently it was slower for OSX to close out the socket than on linux.

@Kubuxu wanna review?

@whyrusleeping whyrusleeping added the need/review Needs a review label Aug 18, 2016
@Kubuxu
Copy link
Member

Kubuxu commented Aug 18, 2016

Can you add me to libp2p org so I can assign myself?

@whyrusleeping
Copy link
Contributor Author

invite sent

@Kubuxu Kubuxu self-assigned this Aug 18, 2016
@whyrusleeping
Copy link
Contributor Author

The one thought I had here is that the preference logic should probably be moved into the peerstore.

@whyrusleeping
Copy link
Contributor Author

@Kubuxu still review this, but i'm gonna merge it so i can move forward here.

@whyrusleeping whyrusleeping merged commit 5013feb into master Aug 23, 2016
@whyrusleeping whyrusleeping deleted the feat/protocol-prefs branch August 23, 2016 04:15
@whyrusleeping whyrusleeping removed the status/in-progress In progress label Aug 23, 2016
@Kubuxu
Copy link
Member

Kubuxu commented Aug 23, 2016

LGTM, it would be worth to use protocol.ID higher in stack instead of string but it isn't that important.

@whyrusleeping
Copy link
Contributor Author

@Kubuxu yeah, i was thinking that too. we can do that later, it was kindof awkward to do the casting between the peerstore and multistream code

@Kubuxu Kubuxu removed their assignment Aug 26, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need/review Needs a review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants