Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

swarm: Extend NetworkBehaviour callbacks. #2011

Merged
merged 5 commits into from
Mar 24, 2021
Merged

swarm: Extend NetworkBehaviour callbacks. #2011

merged 5 commits into from
Mar 24, 2021

Conversation

dvc94ch
Copy link
Contributor

@dvc94ch dvc94ch commented Mar 22, 2021

No description provided.

@dvc94ch dvc94ch changed the title Extend NetworkBehaviour callbacks swarm: Extend NetworkBehaviour callbacks. Mar 22, 2021
romanb
romanb reviewed Mar 23, 2021
View reviewed changes
swarm/src/lib.rs
this.behaviour.inject_new_external_addr(&addr);
}
this.external_addrs.add(addr, score);
this.add_external_address(addr, score);
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

mxinden
mxinden approved these changes Mar 23, 2021
View reviewed changes
Copy link
Member

@mxinden mxinden left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks for following this all the way through @dvc94ch!

@dvc94ch
Copy link
Contributor Author

dvc94ch commented Mar 24, 2021

A release with these changes would be nice for ipfs-embed (as there have been quite a few changes due to using master with Deref/DerefMut being removed etc.)

@mxinden mxinden merged commit 7779b8e into libp2p:master Mar 24, 2021
@mxinden
Copy link
Member

mxinden commented Mar 24, 2021

For the record, commit updating the Swarm changelog: bb206bc

@mxinden
Copy link
Member

mxinden commented Mar 24, 2021

A release with these changes would be nice for ipfs-embed (as there have been quite a few changes due to using master with Deref/DerefMut being removed etc.)

Do I understand correctly that you are already running off of libp2p master? If so, how would a release of libp2p-swarm help? In either case you would have to tackle the breaking changes, no?

(As an aside, the script here might help with the upgrade.)

I am not opposed to doing a new release, it is just a bit of work as it does not only require a new release of libp2p-swarm but all its dependents as well.

@dvc94ch
Copy link
Contributor Author

dvc94ch commented Mar 24, 2021

well, I mean that it helps stop needing master, and because there have been a few changes on master backporting stuff to the current release is a bit annoying.

@tomaka
Copy link
Member

tomaka commented Apr 15, 2021

Would it be possible to get better pull request descriptions in the future? 🙏
Having to read a diff when updating to a new version of libp2p to figure out which methods have been added and changed is a bit annoying, to say the least.

@mxinden
Copy link
Member

mxinden commented Apr 15, 2021

👍 for better pull request descriptions.

To better understand your process @tomaka, did you get here via a git blame or via the CHANGELOG.md entry?

@tomaka tomaka mentioned this pull request Apr 15, 2021
Merged
@thomaseizinger thomaseizinger mentioned this pull request Oct 14, 2022
Merged
4 tasks
@jxs jxs mentioned this pull request Oct 18, 2022
Closed
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@romanb romanb romanb approved these changes

@mxinden mxinden mxinden approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dvc94ch @mxinden @tomaka @romanb