This repository has been archived by the owner on Sep 11, 2024. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add ringing for matrixRTC #11870
Add ringing for matrixRTC #11870
Changes from 7 commits
be80e21
ad26d1d
71263ea
dee9d79
4148d6d
353adc9
db6993e
4359297
59600d0
a8ba346
ca824a4
ba62fc5
478da26
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This type of change is suspicious to me - we should be able to have full trust in our types when we say that
eventType
has typestring
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems like this is not required anymore. I do remember this was a complete crash before. Maybe sth upstream was fixed. I will test a bit more but this should not be necassary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non-blocking, but this really feels like logic that should live in the JS-SDK somehow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CallNotifyContent is declared in the js-sdk.
But I guess:
send_call_notify_event(notify_type: "ring" : "notify", mentions: IMentions, application: string, callId: string)
is what you are proposing? I was considering this but I almost prefer this kind of signature for this amount if fields:send(props: SendData)
vssend(prop1: data1, prop2:data2, ...)
The only thing we could get implicitly is
EventType.CallNotify
. The rest should be configurable anyways.