announced_ip can be a hostname as per spec #1322
Conversation
Fixes #1321 As explained in the ticket, only Rust changes are needed since in Node (and Worker) `announcedIp` was already defined as optional string.
| @@ -76,14 +77,14 @@ pub struct ListenInfo { | |||
| } | |||
|
|
|||
| impl ListenInfo { | |||
| pub(crate) fn to_fbs(self) -> transport::ListenInfo { | |||
| pub(crate) fn to_fbs(&self) -> transport::ListenInfo { | |||
There was a problem hiding this comment.
Rust told me to change this to avoid confusing naming since self is passed as reference after removing Copy derive (or something like that).
rust/src/messages.rs
Outdated
| @@ -644,7 +644,7 @@ impl RouterCreateWebrtcTransportListen { | |||
| web_rtc_transport::ListenIndividual { | |||
| listen_infos: listen_infos | |||
| .iter() | |||
| .map(|listen_info| listen_info.to_fbs()) | |||
| .map(|listen_info| listen_info.clone().to_fbs()) | |||
There was a problem hiding this comment.
Finally (after some other change) this is not needed, so I'm removing it.
It should be However I'd prefer if we don't rename it in Rust now. It would be a bit unexpected to have |
We can also rename it in Node.js, but also keep the old field for backwards compatibility reasons. Ugly, I know, but we can get people upgrade to it already and maybe even print warnings in logs of those who didn't upgrade to the new field name. Not critical or a blocker of course. |
This deserves a separate story since it's huger than expected: |
Fixes #1321
As explained in the ticket, only Rust changes are needed since in Node (and Worker)
announcedIpwas already defined as optional string.