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

[Peer] Adjust choose interface to return a onFinish closure #522

Merged
merged 2 commits into from Dec 8, 2016

Conversation

willhug
Copy link
Contributor

@willhug willhug commented Dec 2, 2016

Summary: in #472 we defined a new interface for having safer onfinish changes. This diff performs the change to the interface

@mention-bot
Copy link

@willhug, thanks for your PR! By analyzing the history of the files in this pull request, we identified @kriskowal, @AlexAPB and @abhinav to be potential reviewers.

Copy link
Contributor

@kriskowal kriskowal left a comment

Choose a reason for hiding this comment

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

Let me know if you decide to cache the finisher and I’ll be glad to rereview. Looks good.

}

for {
if nextPeer := pl.nextPeer(); nextPeer != nil {
pl.notifyPeerAvailable()
return nextPeer, nil
nextPeer.StartRequest(pl)
return nextPeer, pl.getOnFinishFunc(nextPeer), nil
Copy link
Contributor

Choose a reason for hiding this comment

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

We should cache this on the ring next to the node it effects.

@willhug willhug changed the base branch from _start_-adjustchoosepeer-part_1.0 to dev December 8, 2016 21:37
@willhug willhug merged commit 30f39a8 into dev Dec 8, 2016
@willhug willhug deleted the adjustchoosepeer-part_1.0 branch December 8, 2016 21:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants