Skip to content
This repository has been archived by the owner on Jul 21, 2023. It is now read-only.

Make it official that mplex is actually a thing #68

Closed
3 tasks done
daviddias opened this issue Aug 19, 2017 · 10 comments
Closed
3 tasks done

Make it official that mplex is actually a thing #68

daviddias opened this issue Aug 19, 2017 · 10 comments
Assignees
Labels
exp/expert Having worked on the specific codebase is important status/ready Ready to be worked

Comments

@daviddias
Copy link
Member

daviddias commented Aug 19, 2017

The multicodec is mplex, the implementation has diverged greatly from multiplex and currently there is no spec anyway.

I propose:

  • Bring the fork code to this module (including tests)
  • rename to js-libp2p-mplex
  • Write a reverse spec (since code was first)

Me and @Stebalien have been chatting about nuances of multiplex and its feature set and there are a lot of ?, ugh and meh. Another great example is the CR from @whyrusleeping on this PR libp2p/go-mplex#12 (comment) :)

@daviddias
Copy link
Member Author

Nothing against? Otherwise, I'll just go ahead :)

@Stebalien
Copy link
Member

At this point, I'm pretty sure it has converged back. Is this not the case?

@daviddias
Copy link
Member Author

What do you mean by that?

@Stebalien
Copy link
Member

As in, don't both implementations now follow the "multiplex" spec? Wait, which came first, the javascript version or the go version?

@daviddias
Copy link
Member Author

First -> http://npmjs.org/multiplex, Then -> go-multiplex (impl without spec, Then new js-multiplex (built by us, still no spec), Then -> interop achieved.

@Stebalien
Copy link
Member

So, looking at that code, I don't see much divergence.

@daviddias
Copy link
Member Author

daviddias commented Nov 28, 2017

@Stebalien are you sure you are comparing the right codebases?

https://github.com/dignifiedquire/multiplex vs https://github.com/maxogden/multiplex ?

image

Also, the point I'm making here is that today we today use a muxer which is a fork of a fork of a muxer and none has a spec, which makes it really hard to track changes and to have a conversation about how it should work.

Note, the multicodec is already mplex, calling it mplex and spec it out will only bring good things

@Stebalien
Copy link
Member

There's divergence in terms of code but the I don't see any obvious protocol-breaking changes. However, without an original spec, it's hard to tell.

Also, I've now put together an up-to-date reverse spec here: https://github.com/whyrusleeping/go-multiplex/blob/master/spec.md

Yes, I agree that calling it mplex is probably best (to use a less generic term, if for no other reason).

@daviddias daviddias added status/ready Ready to be worked exp/expert Having worked on the specific codebase is important labels Feb 5, 2018
@daviddias
Copy link
Member Author

@daviddias
Copy link
Member Author

Done with #72

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
exp/expert Having worked on the specific codebase is important status/ready Ready to be worked
Projects
None yet
Development

No branches or pull requests

3 participants