Skip to content

[MatrixRTC] Improve is livekit checks to check for field availability and first. #5039

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
Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

[MatrixRTC] Improve is livekit checks to check for field availability and first. #5039

wants to merge 2 commits into from
+21 −2

Conversation

@toger5
Copy link
Contributor

@toger5 toger5 commented Oct 10, 2025 •
edited
Loading

Otherwise passing undefined throws.

Signed-off-by: Timo K [email protected]

Checklist

  • Tests written for new code (and old code if feasible).
  • New or updated public/exported symbols have accurate TSDoc documentation.
  • Linter and other CI checks pass.
  • Sign-off given on the changes (see CONTRIBUTING.md).

@toger5
Improve is livekit checks to check for field
14cc1bb 
availability and first.

Signed-off-by: Timo K <[email protected]>
@toger5
add test
3e24e15 
Signed-off-by: Timo K <[email protected]>
@toger5 toger5 marked this pull request as ready for review October 10, 2025 09:01
@toger5 toger5 requested a review from a team as a code owner October 10, 2025 09:01
@toger5 toger5 requested a review from AndrewFerr October 10, 2025 09:01
@Half-Shot Half-Shot self-requested a review October 27, 2025 13:12
@richvdh
Copy link
Member

richvdh commented Oct 27, 2025 •
edited
Loading

Improve is livekit checks

this doesn't seem to make sense? Should this be "Improve isLivekit checks" or similar?

and first.

and first?

Half-Shot
Half-Shot approved these changes Oct 27, 2025
View reviewed changes
Copy link
Contributor

@Half-Shot Half-Shot left a comment

Choose a reason for hiding this comment

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

This looks like it is doing what it is supposed to do.

Half-Shot
Half-Shot requested changes Oct 27, 2025
View reviewed changes
Copy link
Contributor

@Half-Shot Half-Shot left a comment

Choose a reason for hiding this comment

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

Sorry, missed a bit.

src/matrixrtc/LivekitTransport.ts
livekit_service_url: string;
}

export const isLivekitTransportConfig = (object: any): object is LivekitTransportConfig =>
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, this would be better as unknown. That way Typescript is actually type checking.

src/matrixrtc/LivekitTransport.ts
/**
* @deprecated see LivekitFocusSelection
*/
export const isLivekitFocusSelection = (object: any): object is LivekitFocusSelection =>
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

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

Reviewers

@Half-Shot Half-Shot Half-Shot requested changes

@AndrewFerr AndrewFerr Awaiting requested review from AndrewFerr AndrewFerr is a code owner automatically assigned from matrix-org/element-call-reviewers

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

T-Defect

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@toger5 @richvdh @Half-Shot