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

fix: remove stream add/remove methods from connection interface #1912

Merged
merged 1 commit into from
Aug 7, 2023

Conversation

tabcat
Copy link
Contributor

@tabcat tabcat commented Jul 31, 2023

BEGIN_COMMIT_OVERRIDE
fix!: remove stream add/remove methods from connection interface

addStream and removeStream are internal methods that do not need to be part of the public API.

They are called internally on the connection implementation by the upgrader and do not need to be exposed to end users.

Closes #1855

BREAKING CHANGE: the addStream and removeStream methods have been removed from the connection interface
END_COMMIT_OVERRIDE

@tabcat
Copy link
Contributor Author

tabcat commented Jul 31, 2023

should this be a breaking change?

@tabcat
Copy link
Contributor Author

tabcat commented Jul 31, 2023

These methods are more prevalent than I thought. Going to close for now and maybe open this or another pr later.

@tabcat tabcat closed this Jul 31, 2023
@tabcat tabcat reopened this Aug 5, 2023
@tabcat tabcat force-pushed the connection-interface branch 3 times, most recently from 20c6680 to 1479ca3 Compare August 5, 2023 15:28
@maschad maschad marked this pull request as draft August 5, 2023 16:29
@maschad
Copy link
Member

maschad commented Aug 5, 2023

I've converted it to a draft to avoid unnecessary notifications, feel free to re-open once you feel it ready for review.

@tabcat tabcat force-pushed the connection-interface branch 4 times, most recently from c9f0ab4 to 451cb19 Compare August 7, 2023 13:11
@tabcat tabcat marked this pull request as ready for review August 7, 2023 15:31
@tabcat tabcat force-pushed the connection-interface branch from 451cb19 to d8cee9d Compare August 7, 2023 16:06
remove (add|remove)Stream from connection interface

remove (add|remove)Stream use from interface-compliance-tests

remove (add|remove)Stream from libp2p connection methods

remove (add|remove)Stream from libp2p upgrader methods

remove ts-sinon dependency from interface-compliance-tests
@tabcat tabcat marked this pull request as draft August 7, 2023 16:57
@tabcat tabcat force-pushed the connection-interface branch from d8cee9d to 1fe698b Compare August 7, 2023 17:12
@tabcat tabcat marked this pull request as ready for review August 7, 2023 17:43
Copy link
Member

@maschad maschad left a comment

Choose a reason for hiding this comment

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

lgtm

@maschad maschad changed the title fix: Remove Stream methods from Connection fix: remove stream add/remove methods from connection interface Aug 7, 2023
@maschad maschad merged commit e26848b into libp2p:master Aug 7, 2023
@achingbrain
Copy link
Member

@maschad because methods were removed from a public API this is a breaking change.

I have updated the issue description to use a commit override.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove Stream methods from Connection interface
3 participants