rust-libp2p icon indicating copy to clipboard operation
rust-libp2p copied to clipboard
verified publisher icon libp2p

*: Remove default features from all crates

Open thomaseizinger opened this issue 3 years ago • 6 comments

Description

This patch-set removes our use of default features. We also introduce a new full feature that activates all features.

Links to any relevant issues

  • https://github.com/libp2p/rust-libp2p/pull/2173
  • https://github.com/libp2p/rust-libp2p/pull/2146

Open Questions

Change checklist

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

thomaseizinger avatar Sep 20 '22 03:09 thomaseizinger

Given that this is a large breaking change for users, is there anyone objecting to this change?

Is it that large a change? Activate a couple of feature flags and your back in. No functionality is changing!

thomaseizinger avatar Sep 22 '22 09:09 thomaseizinger

Given that this is a large breaking change for users, is there anyone objecting to this change?

Is it that large a change? Activate a couple of feature flags and your back in. No functionality is changing!

I remember tokio moving to this convention taking me a couple of h. I am not saying it is hard. Just saying it is tedious. Never the less I am in favor of this change.

@thomaseizinger do you want this to be included in v0.49.0 (https://github.com/libp2p/rust-libp2p/pull/2931)?

mxinden avatar Sep 22 '22 16:09 mxinden

Given that this is a large breaking change for users, is there anyone objecting to this change?

Is it that large a change? Activate a couple of feature flags and your back in. No functionality is changing!

I remember tokio moving to this convention taking me a couple of h. I am not saying it is hard. Just saying it is tedious. Never the less I am in favor of this change.

@thomaseizinger do you want this to be included in v0.49.0 (#2931)?

That would be great, yeah!

thomaseizinger avatar Sep 23 '22 04:09 thomaseizinger

I would be great to have a short section "How to upgrade" for the release, so folks now how to adjust their feature usage.

dignifiedquire avatar Sep 23 '22 09:09 dignifiedquire

I would be great to have a short section "How to upgrade" for the release, so folks now how to adjust their feature usage.

Is the first sentence in the changelog entry not good enough? https://github.com/libp2p/rust-libp2p/blob/2173-no-default-features/CHANGELOG.md#0490---unreleased

thomaseizinger avatar Sep 23 '22 10:09 thomaseizinger

Is the first sentence in the changelog entry not good enough?

I somehow missed that when looking through the diff. LGTM 👍

dignifiedquire avatar Sep 23 '22 11:09 dignifiedquire

@thomaseizinger would you mind resolving the merge conflicts here? No rush, just a ping.

mxinden avatar Sep 27 '22 14:09 mxinden

@thomaseizinger would you mind resolving the merge conflicts here? No rush, just a ping.

Sure! I just didn't realize there are any :/ I opened https://github.com/libp2p/rust-libp2p/issues/2948.

thomaseizinger avatar Sep 28 '22 00:09 thomaseizinger

@mxinden Resolved.

thomaseizinger avatar Sep 28 '22 00:09 thomaseizinger

Wooooo! This is so exciting!

thomaseizinger avatar Sep 29 '22 22:09 thomaseizinger

As long as #2950 is still open, this could result in a very confusing experience for a beginner. We should definitely fix that.

umgefahren avatar Oct 04 '22 08:10 umgefahren