Skip to content
This repository has been archived by the owner on Aug 19, 2022. It is now read-only.

refactor interfaces #9

Merged
merged 2 commits into from
Jun 21, 2019
Merged

refactor interfaces #9

merged 2 commits into from
Jun 21, 2019

Conversation

magik6k
Copy link
Contributor

@magik6k magik6k commented Jun 21, 2019

No description provided.

Copy link
Member

@raulk raulk left a comment

Choose a reason for hiding this comment

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

noticed a tiny bug; I'll push a commit to fix it. Other than that, LGTM.

EDIT: nevermind, apparently I cannot read negative ifs.

basic.go Show resolved Hide resolved
basic.go Show resolved Hide resolved
}

for _, ch := range n.sinks {
ch.Send(eval)
ch <- event
Copy link
Member

Choose a reason for hiding this comment

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

We'll soon want to handle backpressure here, but it's fine for now.

@raulk
Copy link
Member

raulk commented Jun 21, 2019

Rectified my review above; apparently I can't read code on a Friday afternoon 😛 I'll release go-libp2p-core, then update go.mod here and merge.

@raulk raulk changed the title Refactor interfaces refactor interfaces Jun 21, 2019
@raulk raulk merged commit 6212a92 into master Jun 21, 2019
@raulk raulk deleted the feat/own-channels branch June 21, 2019 16:50
typ := reflect.TypeOf(etyp)

if typ.Kind() != reflect.Ptr {
return nil, errors.New("subscribe called with non-pointer type")
Copy link
Member

Choose a reason for hiding this comment

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

This needs to close already created subscriptions.

Copy link
Member

Choose a reason for hiding this comment

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

👍 yeah, since this is merged and released, could you open an issue for it?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants