docs: add archive notice to webrtc star and direct #1488
Conversation
|
Can we please only archive these things when there's an actual alternative for people to use? Right now @libp2p/webrtc-star is the only way to do browser to browser communication - once the @libp2p/webrtc transport has the browser to browser component done, then we can archive WebRTC-Star but given it hasn't even made it on to npm yet this feels a little premature. |
|
@achingbrain I see your point, the alternatives don't exist or aren't mature enough. However, the primary motivation to archive was to alleviate us of further maintenance work as that's sunken cost (this includes triaging new issues) and let the community know our intention. If you feel this is a bit too soon, I don't see any problem unarchiving them for now. cc: @BigLep as he was also keen to archive the older solutions |
|
@BigLep poke. we discussed this in last weeks SitRep but can you confirm here as well. |
|
I agree it stinks to pull things away from users, but it's being truthful. It's not a priority for the js-libp2p maintainers to support it. We shouldn't spend time triaging or supporting it. Lets put that effort into the general WebRTC effort. I think the message we want to get across crystal clear is "use at your own risk. This is not maintained. Here is what we're working on to replace it." I think we're doing this, but if we need to message this better, let's do it. That said, @p-shahi it looks like npm hasn't been updated to make clear this is deprecated: https://www.npmjs.com/package/@libp2p/webrtc-star Can we please do that as well? |
|
Given that we released browser to browser in https://github.com/libp2p/js-libp2p-webrtc/releases/tag/v1.1.0 we should be good to merge this once https://www.npmjs.com/package/@libp2p/webrtc-star is deprecated on npm @p-shahi |
achingbrain
changed the title
Add new transports to the browser example and remove the outdated webrtc-direct example as it uses a deprecated transport. Refs: #1488
|
@maschad Can you take over this PR? i.e. replace the customizing transports with your suggestion |
| - [@chainsafe/discv5](https://github.com/chainsafe/discv5) | ||
|
|
||
| **Note**: `peer-discovery` services within transports (such as `js-libp2p-webrtc-star`) are automatically gathered from the `transport`, via it's `discovery` property. As such, they do not need to be added in the discovery modules. However, these transports can also be configured and disabled as the other ones. |
There was a problem hiding this comment.
This is still true of modules like kad-dht so might be worth mentioning here.
There was a problem hiding this comment.
I think the note is a bit confusing overall, since we configure our discovery service via a services module now, intuitively one would configure that separately from the transport.
So I think it's best to exclude this note.
| webRTC.discovery | ||
| webRTC({ | ||
| dataChannel: { | ||
| maxMessageSize: 10 |
There was a problem hiding this comment.
Why is this config key necessary? People will copy/paste this as good config so it's worth noting why we're overriding the default value here. If the default value is wrong it should probably be changed instead.
There was a problem hiding this comment.
The purpose of this example was to hihglight that you can pass an optional maxMessageSize value as mentioned in the above paragraph.
|
I've move the webRTC, websocket and webtransport examples to their own subdirectories within the |
Relates to: libp2p/github-mgmt#80