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.

Sign up for GitHub

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

Jump to bottom

Remove Stream methods from Connection interface #1855

Closed
maschad opened this issue Mar 28, 2023 · 2 comments · Fixed by #1912
Closed

Remove Stream methods from Connection interface #1855

maschad opened this issue Mar 28, 2023 · 2 comments · Fixed by #1912
Labels
exp/beginner Can be confidently tackled by newcomers good first issue Good issue for new contributors help wanted Seeking public contribution on this issue

Comments

@maschad
Copy link
Member

maschad commented Mar 28, 2023

Following up from the discussion here the muxers should be ultimately responsible for managing streams as opposed to the connections themselves
@maschad maschad added good first issue Good issue for new contributors exp/beginner Can be confidently tackled by newcomers help wanted Seeking public contribution on this issue labels Mar 28, 2023
@mmsaki mmsaki mentioned this issue Jun 15, 2023
Closed
@achingbrain achingbrain transferred this issue from libp2p/js-libp2p-interfaces Jun 26, 2023
tabcat added a commit to tabcat/js-libp2p that referenced this issue Jul 31, 2023
@tabcat
fix: Remove Stream methods from Connection
3c1c6f8 
remove (add|remove)Stream from connection interface

Related: libp2p#1855
@tabcat tabcat mentioned this issue Jul 31, 2023
Merged
@tabcat
Copy link
Contributor

tabcat commented Jul 31, 2023

I'm going to look a bit closer but seems like this requires moving the functionality of what is currently (add|remove)Stream and putting that on the muxer interface?

@maschad
Copy link
Member Author

maschad commented Jul 31, 2023

There wouldn't be any use for adding those methods to the stream muxer interface since we already have the newStream and close methods so these methods just need to be removed from the connection interface.

@mmsaki actually began work on this on #1829 but I am not sure what the status of that is now. Perhaps they can provide an update.

tabcat added a commit to tabcat/js-libp2p that referenced this issue Aug 5, 2023
@tabcat
fix: Remove Stream methods from Connection
b537440 
remove (add|remove)Stream from connection interface

Related: libp2p#1855

expect .addStream ts error
tabcat added a commit to tabcat/js-libp2p that referenced this issue Aug 5, 2023
@tabcat
fix: Remove Stream methods from Connection
20c6680 
remove (add|remove)Stream from connection interface

Related: libp2p#1855

expect .addStream ts error

handle (add|remove)Stream usage
tabcat added a commit to tabcat/js-libp2p that referenced this issue Aug 5, 2023
@tabcat
fix: Remove Stream methods from Connection
1479ca3 
remove (add|remove)Stream from connection interface

Related: libp2p#1855

comment out .addStream test usage

handle (add|remove)Stream usage
@github-project-automation github-project-automation bot moved this from 🛠️ Todo to 🎉Done in js-libp2p Aug 7, 2023
This was referenced Jan 18, 2024
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
exp/beginner Can be confidently tackled by newcomers good first issue Good issue for new contributors help wanted Seeking public contribution on this issue
Projects
js-libp2p
Archived in project
Milestone
No milestone
2 participants
@maschad @tabcat