search

multiformats / rust-multiaddr

multiaddr implementation in rust
https://crates.io/crates/multiaddr
Other
86 stars 45 forks source link

*: Upstream parity-multiaddr changes #40

Closed mxinden closed 3 years ago

mxinden commented 3 years ago

parity-multiaddr is a fork of this repository with various changes and additions. As part of https://github.com/libp2p/rust-libp2p/issues/758 this pull request does the following in order to merge the two while preserving the git history of both repositories:

Main merge happens via https://github.com/multiformats/rust-multiaddr/pull/40/commits/935a9664b4b12048fc0babbe734e69923b489a28. As far as I can tell all features of multiaddr and parity-multiaddr are preserved. Naming closely follows parity-multiaddr, thus crates depending on multiaddr (as far as I can tell none) will face breaking changes. In case of conflicts parity-multiaddr is favored over multiaddr given that the former is more up-to-date and has a larger user base. Version is fast-forwarded to v0.12.0 to not conflict with existing parity-multiaddr versions.

mxinden commented 3 years ago

@vmx @Stebalien does one of you have some time to give this pull request a review? I am sorry for the rather large change-set. Let me know in case you see a way to simplify the review work.

Stebalien commented 3 years ago

In a few weeks.

mxinden commented 3 years ago

One thing that might be worth mentioning in the changelog/commit message is that you cannot any longer convert from an integer to a Protocol and back, though you have constants to get the integer values of protocols.

Addressed in https://github.com/multiformats/rust-multiaddr/pull/40/commits/11d6fcf5a359c38c101c46665f18b215b844528d.

It is not Copy any more, it might be worth mentioning in the changelog/commit message.

Addressed in https://github.com/multiformats/rust-multiaddr/pull/40/commits/11d6fcf5a359c38c101c46665f18b215b844528d.

@vmx thank you for the review. Would you mind taking another look?

mxinden commented 3 years ago

In a few weeks.

@Stebalien I am interpreting this as an offer, not as a request to pause this pull request till then. Let me know if you would prefer to pause this pull request until you find time to review, after all this effort is in no rush.

Unless anything comes up, I will merge this sometime next week and cut the v0.12.0 release. Thanks @vmx for the help.

Stebalien commented 3 years ago

I am interpreting this as an offer, not as a request to pause this pull request till then.

Ah, no. It was an offer to ping me in a few weeks if this got stuck. Thanks @vmx!