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

feat(core): deprecate upgrade::from_fn #3747

Merged
merged 4 commits into from
Apr 19, 2023
Merged

Conversation

thomaseizinger
Copy link
Contributor

Description

This functionality isn't needed anywhere in rust-libp2p so we can deprecate and later remove it and thus reduce our API surface. Users relying on this function can vendor it.

Notes & open questions

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

@thomaseizinger thomaseizinger requested a review from mxinden April 6, 2023 19:24
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 strong opinion here. Does this block any other patches? I don't think it requires much maintenance, does it?

@thomaseizinger
Copy link
Contributor Author

It is API surface that I'd rather not have unless really necessary. When taking apart libp2p-core, the from_fn upgrade was one of those things where I wasn't sure where to put them otherwise.

@thomaseizinger
Copy link
Contributor Author

It will also uses upgrade::apply in this docs which I want to make an implementation detail of transport upgrades, again to reduce API surface. See #3748.

@mergify mergify bot merged commit 0fe9791 into master Apr 19, 2023
@mergify mergify bot deleted the feat/remove-from-fn-upgrade branch April 19, 2023 22:07
mergify bot pushed a commit that referenced this pull request Apr 26, 2023
This is dead-code, we don't use it in our codebase. I am suggesting to remove it because it creates an unnecessary API surface.

Related: #3747.
Related: #3271.

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

Successfully merging this pull request may close these issues.

2 participants