Skip to content
This repository has been archived by the owner on May 13, 2024. It is now read-only.

Karkakol/add simulcast config to endpoints #55

Merged
merged 18 commits into from
Jan 25, 2024

Conversation

karkakol
Copy link
Member

No description provided.

@karkakol karkakol requested review from graszka22 and incubo4u and removed request for graszka22 January 16, 2024 15:51
Copy link
Contributor

@graszka22 graszka22 left a comment

Choose a reason for hiding this comment

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

Also most of the comments from android side apply here

<?xml version="1.0" encoding="UTF-8"?>
<module type="WEB_MODULE" version="4">
<component name="NewModuleRootManager">
<content url="file://$MODULE_DIR$">
Copy link
Contributor

Choose a reason for hiding this comment

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

don't we need to gitignore this stuff too?

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@@ -30,12 +30,12 @@ public protocol ReceivableEvent {
var type: ReceivableEventType { get }
}

internal struct ReceivableEventBase: Decodable {
public struct ReceivableEventBase: Decodable {
Copy link
Contributor

Choose a reason for hiding this comment

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

why public? it was intentionally internal in order to not expose this to the user and to generate docs nicely
this also applies to the other places

@@ -3,49 +3,59 @@ public struct Endpoint: Codable {
public let type: String
public let metadata: Metadata
public let trackIdToMetadata: [String: Metadata]?
public let tracks: [String: TracksAddedEvent.Data.TrackData]?
Copy link
Contributor

Choose a reason for hiding this comment

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

wouldn't it be easier if we had no ? here? it can be always an empty array if there are no tracks?

Copy link
Member Author

Choose a reason for hiding this comment

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

i will ask if tracks can be null to JF backend devs

@karkakol karkakol requested a review from graszka22 January 24, 2024 19:39
@karkakol karkakol merged commit d860181 into master Jan 25, 2024
1 check passed
@karkakol karkakol deleted the karkakol/add_simulcast_config_to_endpoints branch March 26, 2024 15:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants