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(swarm): rename NetworkBehaviourAction to ToSwarm #3658

Merged
merged 5 commits into from
Mar 24, 2023

Conversation

thomaseizinger
Copy link
Contributor

Description

Resolves #3123.

Notes & open questions

Do we need to bump the versions of all dependents here? It is backwards-compatible, meaning we can patch-release libp2p-swarm and all dependencies should still be working, even if the new version is pulled in.

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • A changelog entry has been made in the appropriate crates

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.

No need to bump the other crates in my eyes.

@mergify mergify bot merged commit dcbc04e into master Mar 24, 2023
@mergify mergify bot deleted the feat/rename-network-behaviour-action branch March 24, 2023 13:43
@@ -65,7 +65,7 @@ use libp2p_core::{Endpoint, Multiaddr};
use libp2p_identity::PeerId;
use libp2p_swarm::{
dummy, CloseConnection, ConnectionDenied, ConnectionId, FromSwarm, NetworkBehaviour,
NetworkBehaviourAction, PollParameters, THandler, THandlerInEvent, THandlerOutEvent,
PollParameters, THandler, THandlerInEvent, THandlerOutEvent, ToSwarm,
Copy link
Member

Choose a reason for hiding this comment

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

With this change libp2p-allow-block-list depends on libp2p-swarm >=0.42.1, but its Cargo.toml allows libp2p-swarm v0.42.0.

I don't think this is a pressing issue, thus not sure whether to release a patch release of libp2p-allow-block-list with the above restriction. What do you think @thomaseizinger?

The same applies to other crates below that we have released since.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, it is an issue. We should have bumped the dependency version but not the version of each dependent crate.

I'll send a fix.

Can we mitigate this with an automated check somehow? Perhaps -Z minimal-versions helps?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done here: #3711.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can we mitigate this with an automated check somehow? Perhaps -Z minimal-versions helps?

I think I found a solution: #3715

mergify bot pushed a commit that referenced this pull request Apr 5, 2023
With #3658, these crates depend on the `0.42.1` release to access the new `ToSwarm` type. With the currently specified version, a user could theoretically run into a compile error if they pin `libp2p-swarm` to `0.42.0` in their lockfile but update to the latest patch release of one of these crates.

Pull-Request: #3711.
mergify bot pushed a commit that referenced this pull request May 2, 2023
Previously, we would specify the version and path of our workspace dependencies in each of our crates. This is error prone as #3658 (comment) for example shows. Problems like these happened in the past too.

There is no need for us to ever depend on a earlier version than the most current one in our crates. It thus makes sense that we manage this version in a single place.

Cargo supports a feature called "workspace inheritance" which allows us to share a dependency declaration across a workspace and inherit it with `{ workspace = true }`.

We do this for all our workspace dependencies and for the MSRV.

Resolves #3787.

Pull-Request: #3715.
umgefahren pushed a commit to umgefahren/rust-libp2p that referenced this pull request Mar 8, 2024
umgefahren pushed a commit to umgefahren/rust-libp2p that referenced this pull request Mar 8, 2024
With libp2p#3658, these crates depend on the `0.42.1` release to access the new `ToSwarm` type. With the currently specified version, a user could theoretically run into a compile error if they pin `libp2p-swarm` to `0.42.0` in their lockfile but update to the latest patch release of one of these crates.

Pull-Request: libp2p#3711.
umgefahren pushed a commit to umgefahren/rust-libp2p that referenced this pull request Mar 8, 2024
Previously, we would specify the version and path of our workspace dependencies in each of our crates. This is error prone as libp2p#3658 (comment) for example shows. Problems like these happened in the past too.

There is no need for us to ever depend on a earlier version than the most current one in our crates. It thus makes sense that we manage this version in a single place.

Cargo supports a feature called "workspace inheritance" which allows us to share a dependency declaration across a workspace and inherit it with `{ workspace = true }`.

We do this for all our workspace dependencies and for the MSRV.

Resolves libp2p#3787.

Pull-Request: libp2p#3715.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

swarm: Rename NetworkBehaviourAction to ToSwarm
2 participants