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

fix: Rename webrtc-w3c to webrtc-private-to-private #151

Closed
wants to merge 1 commit into from
Closed

fix: Rename webrtc-w3c to webrtc-private-to-private #151

wants to merge 1 commit into from

Conversation

mxinden
Copy link
Member

@mxinden mxinden commented Mar 9, 2023 •
edited
Loading

Summary

We have decided to rename webrtc-w3c to webrtc-private-to-private. See rational in #150.

No software has been released using webrtc-w3c, thus renaming is not a breaking change.

Before Merge

@mxinden
fix: Rename webrtc-w3c to webrtc-private-to-private
dafd97e 
We have decided to rename `webrtc-w3c` to `webrtc-private-to-private`. See
rational in multiformats#150.

No software has been released using `webrtc-w3c`, thus renaming is not a
breaking change.
@mxinden mxinden mentioned this pull request Mar 9, 2023
Merged
@mxinden
Copy link
Member Author

mxinden commented Mar 13, 2023

Friendly ping @achingbrain, @MarcoPolo and @marten-seemann.

@achingbrain achingbrain mentioned this pull request Mar 13, 2023
Merged
@p-shahi p-shahi mentioned this pull request Mar 13, 2023
Merged
4 tasks
@achingbrain
Copy link
Member

Cross posting my comment from here as it's a closed issue:

Would it be fair to say that what we're calling /webrtc-private-to-private, if we can't call it /webrtc-w3c (which, on balance, is not a completely terrible name since it uses the w3c flow and the spec mentions the w3c APIs specifically) is really something like /webrtc+w3c or /webrtc+w3c-flow or something, using a + to differentiate between the transport and our connection protocol we place on top of it, and what we're calling /webrtc is really /webrtc+sdp-munging?

I realise it's the 11th hour, but it seems that /webrtc-private-to-private is not a transport in the same way that /tcp or /ws is a transport - the transport is webrtc and private-to-private is a way of using that transport. (an aside: I'm not sure what private means in this context? Vs public? Undiallable? Webrtc makes browser nodes diallable. I am at a loss.)

The browser-to-server /webrtc multiaddr segment name seems to imply there's nothing special or magic about it, so a fresh pair of eyes may assume you can implement it with any spec-compliant webrtc implementation, but this is not the case as it uses sdp munging which is our own spec notes is disallowed by the webrtc spec and as such is unimplementable in any spec-compliant environment.

That's fine and all, but the unadorned /webrtc seems a bit misleading.

achingbrain
achingbrain requested changes Mar 14, 2023
View reviewed changes
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.

Not that I want to put words in @marten-seemann's mouth, but from a different channel:

achingbrain So you'd be happy calling browser to browser /webrtc and browser to server /webrtc+sdp-munging?
Marten Seemann Yes. I actually like calling out that we’re doing the munging
Because that’s the crucial difference between the two

This makes it clear that a spec compliant WebRTC implementation can use this address to communicate, and ones that are more flexible can let peers know that they can use SDP munging via a different address.

protocols.csv
@@ -36,6 +36,6 @@ code, size, name, comment
275, 0, p2p-webrtc-star,
276, 0, p2p-webrtc-direct,
280, 0, webrtc, ICE-lite webrtc transport
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
280, 0, webrtc, ICE-lite webrtc transport
280, 0, webrtc+sdp-munging, ICE-lite WebRTC transport with SDP munging

protocols.csv
@@ -36,6 +36,6 @@ code, size, name, comment
275, 0, p2p-webrtc-star,
276, 0, p2p-webrtc-direct,
280, 0, webrtc, ICE-lite webrtc transport
281, 0, webrtc-w3c, webrtc transport where connection establishment is according to w3c spec
281, 0, webrtc-private-to-private, WebRTC transport establishing connection between two private nodes
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
281, 0, webrtc-private-to-private, WebRTC transport establishing connection between two private nodes
281, 0, webrtc, WebRTC transport establishing connection between two WebRTC spec compliant nodes

achingbrain added a commit that referenced this pull request Mar 14, 2023
@achingbrain
fix: Rename webrtc-w3c to webrtc and webrtc to webrtc-direct
f7e746e 
Following on from #150 and an replacement for #151

Renames:

- `/webrtc-w3c` -> `/webrtc` - AKA browser to browser
- `/webrtc` -> `/webrtc-direct` - AKA browser to server

Discussion:

- This option was mentioned in #150 and seemed to have a reasonable amount of support but seemed to get lost amongst the other options
- The differences in the protocols is mentioned in the table comments
@achingbrain achingbrain mentioned this pull request Mar 14, 2023
Merged
2 tasks
achingbrain added a commit that referenced this pull request Mar 14, 2023
@achingbrain
fix: Rename webrtc-w3c to webrtc and webrtc to webrtc+sdp-munging
c7104b7 
Following on from #150 and an replacement for #151

Renames:

- `/webrtc-w3c` -> `/webrtc` - AKA browser to browser
- `/webrtc` -> `/webrtc+sdp-munging` - AKA browser to server

Discussion:

- This option comes from comments on #151
- It got a lukewarm reception on the triage call so it's presented as an option along with #152
- Adding `+sdp-munging` makes it more explicit about the differences, though admittedly there are other differences that aren't encapsulated in the name
- People were uncertain about universal understanding of the term "SDP munging"
@achingbrain achingbrain mentioned this pull request Mar 14, 2023
Closed
2 tasks
achingbrain added a commit that referenced this pull request Mar 17, 2023
@achingbrain
fix: Rename webrtc-w3c to webrtc and webrtc to webrtc-direct (#152)
2ad868a 
Following on from #150 and an replacement for #151

Renames:

- `/webrtc-w3c` -> `/webrtc` - AKA browser to browser
- `/webrtc` -> `/webrtc-direct` - AKA browser to server

Discussion:

- This option was mentioned in #150 and seemed to have a reasonable amount of support but seemed to get lost amongst the other options
- The differences in the protocols is mentioned in the table comments
@achingbrain
Copy link
Member

Closing in favour of #152

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@achingbrain achingbrain achingbrain requested changes

@MarcoPolo MarcoPolo MarcoPolo approved these changes

@marten-seemann marten-seemann Awaiting requested review from marten-seemann

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mxinden @achingbrain @MarcoPolo