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

Don't update calls with no livekit URL #3598

Merged
merged 3 commits into from
Jul 12, 2023
Merged

Don't update calls with no livekit URL #3598

merged 3 commits into from
Jul 12, 2023

Conversation

dbkr
Copy link
Member

@dbkr dbkr commented Jul 12, 2023 •
edited by github-actions bot
Loading

Expose method to update it instead and generally simplify a bit: change it to a single string rather than an array of structs.

Checklist

  • Tests written for new code (and old code if feasible)
  • Linter and other CI checks pass
  • Sign-off given on the changes (see CONTRIBUTING.md)

Here's what your changelog entry will look like:

🐛 Bug Fixes

  • Don't update calls with no livekit URL (#3598).

@dbkr
Don't update calls with no livekit URL & expose method to update it i…
d4d73a6 
…nstead

and generally simplify a bit: change it to a single string rather than
an array of structs.
@dbkr dbkr added the T-Defect label Jul 12, 2023
@dbkr dbkr requested a review from a team as a code owner July 12, 2023 15:50
dbkr added a commit to element-hq/element-call that referenced this pull request Jul 12, 2023
@dbkr
Update livekit service URLs if they fail
906508a 
Requires matrix-org/matrix-js-sdk#3598
Fixes #1190
Fixes element-hq/matrix-hosted#8021
robintown
robintown reviewed Jul 12, 2023
View reviewed changes
Copy link
Member

@robintown robintown left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This appears to only set the URL on creation, but didn't we also discuss wanting to update it on join?

src/webrtc/groupCall.ts

public get foci(): FocusInfo[] {
return this._foci;
public updateLivekitServiceURL(newURL: string): Promise<void> {
Copy link
Member

Choose a reason for hiding this comment

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

Is this used anywhere?

Copy link
Member

Choose a reason for hiding this comment

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

Oh I see, it's public and can be used in Element Call

dbkr added a commit to element-hq/element-call that referenced this pull request Jul 12, 2023
@dbkr
Update livekit service URLs if they fail
4fe6ac6 
Requires matrix-org/matrix-js-sdk#3598
Fixes #1190
Fixes element-hq/matrix-hosted#8021
@dbkr dbkr mentioned this pull request Jul 12, 2023
Merged
@dbkr dbkr merged commit b698217 into livekit Jul 12, 2023
@dbkr dbkr deleted the dbkr/update_lk_urls branch July 12, 2023 16:31
@robintown robintown mentioned this pull request Jul 12, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@robintown robintown robintown approved these changes

Assignees
No one assigned
Labels
T-Defect
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dbkr @robintown