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!: Rename WebRTCDirect to P2PWebRTCDirect and deprecate #146

Merged

Conversation

ckousik
Copy link
Contributor

@ckousik ckousik commented Mar 17, 2023

Rename webrtc protocol from webrtc to webrtc-direct based on multiformats/multiaddr#152 .

BREAKING CHANGE: the existing WebRTCDirect multicodec name has been deprecated and renamed P2PWebRTCDirect. The new WebRTCDirect codec has been added but is not the same, please check your code before upgrading! Integer codes are unchanged.

Rename webrtc protocol from webrtc to webrtc-direct.
Copy link
Member

@achingbrain achingbrain left a comment

Choose a reason for hiding this comment

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

This PR puts us in an awkward position. We need to support parsing four types of webrtc address:

  • .../webrtc/...
  • .../webrtc-direct/...
  • .../p2p-webrtc-direct/...
  • .../p2p-webrtc-star/...

As it stands, this PR makes WebRTC correspond to the /webrtc-direct address which I think will lead to chaos.

Unfortunately we already export a WebRTCDirect symbol, but thankfully the transport itself is little used.

I think we should rename it to P2PWebRTCDirect and ensure it's mentioned in the release notes. Hopefully this will lead to slightly less chaos.

So:

  • .../webrtc/... - WebRTC
  • .../webrtc-direct/... - WebRTCDirect
  • .../p2p-webrtc-direct/... - P2PWebRTCDirect
  • .../p2p-webrtc-star/... - P2PWebRTCStar

We should also add @deprectated JSDocs to the P2PWebRTCDirect and P2PWebRTCStar symbols.

@ckousik
Copy link
Contributor Author

ckousik commented Mar 17, 2023

@achingbrain I've made the requested changes. The new WebRTC symbol is the browser-to-browser equivalent, while WebRTCDirect is browser-to-server. I've allowed matches where /webrtc is preceded by either a circuit address or a Reliable transport.

@achingbrain achingbrain changed the title fix: Webrtc protocol rename fix!: Rename WebRTCDirect to P2PWebRTCDirect and deprecate Mar 17, 2023
@achingbrain achingbrain merged commit 92f92d7 into multiformats:master Mar 20, 2023
github-actions bot pushed a commit that referenced this pull request Mar 20, 2023
## [12.0.0](v11.1.2...v12.0.0) (2023-03-20)

### ⚠ BREAKING CHANGES

* the existing WebRTCDirect multicodec name has been deprecated and renamed P2PWebRTCDirect. The new WebRTCDirect codec has been added but is not the same, please check your code before upgrading!  Integer codes are unchanged.

### Bug Fixes

* Rename WebRTCDirect to P2PWebRTCDirect and deprecate ([#146](#146)) ([92f92d7](92f92d7))
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.

2 participants