Skip to content

Conversation

@Rados13
Copy link
Contributor

@Rados13 Rados13 commented Apr 5, 2024

Acknowledging the stipulations set forth:

  • I hereby confirm that a Pull Request involving updates to the Software Development Kit (SDK) has been smoothly merged, currently awaits processing, or is otherwise deemed unnecessary in this context.
  • I also affirm that another Pull Request, specifically addressing updates to the documentation body (commonly referred to as 'docs'), has either been successfully incorporated, is in the process of review, or is considered superfluous under the prevailing circumstances.

PR in Elixir SDK
PR in Python SDK
PR in Docs

@Rados13 Rados13 self-assigned this Apr 5, 2024
@Rados13 Rados13 changed the title Recording manual subscribe mode RTC-510 Recording manual subscribe mode Apr 5, 2024
@codecov
Copy link

codecov bot commented Apr 5, 2024

Codecov Report

Merging #174 (f34722d) into main (e9a5f3c) will decrease coverage by 0.35%.
The diff coverage is 95.23%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #174      +/-   ##
==========================================
- Coverage   87.31%   86.97%   -0.35%     
==========================================
  Files          72       72              
  Lines        1388     1397       +9     
==========================================
+ Hits         1212     1215       +3     
- Misses        176      182       +6     
Files Coverage Δ
lib/jellyfish/component/recording.ex 96.87% <100.00%> (+4.01%) ⬆️
lib/jellyfish_web/api_spec/component/recording.ex 100.00% <ø> (ø)
...llyfish_web/controllers/subscription_controller.ex 90.90% <100.00%> (+2.02%) ⬆️
lib/jellyfish_web/router.ex 100.00% <100.00%> (ø)
lib/jellyfish/room.ex 83.26% <90.90%> (-2.17%) ⬇️

... and 1 file with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e9a5f3c...f34722d. Read the comment docs.

@Rados13 Rados13 marked this pull request as ready for review April 5, 2024 09:31
@Rados13 Rados13 force-pushed the recording_manual_subscribe_mode branch 5 times, most recently from 1b8572c to 85dd78c Compare April 5, 2024 14:47
@Rados13 Rados13 requested review from Karolk99 and sgfn April 8, 2024 06:15
""")

with {:ok, serialized_opts} <- serialize_options(options, Options.schema()),
result_opts <- Map.update!(serialized_opts, :subscribe_mode, &String.to_atom/1),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: I would wrap this line in function get_subscribe_mode or parse_subscribe_mode for consistency

end

defp get_hls_component(%{components: components}),
defp get_component_by_id(%{components: components} = _state, component_id),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
defp get_component_by_id(%{components: components} = _state, component_id),
defp get_component_by_id(%{components: components}, component_id),

@Rados13 Rados13 force-pushed the recording_manual_subscribe_mode branch from 4c52688 to 83c7a6c Compare April 8, 2024 09:22
@Rados13 Rados13 requested a review from sgfn April 9, 2024 10:55
"id" => id,
"type" => "recording",
"properties" => %{"pathPrefix" => path_prefix}
"properties" => path_prefix
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this variable be named properties?

@Rados13 Rados13 merged commit 7980db9 into main Apr 9, 2024
@Rados13 Rados13 deleted the recording_manual_subscribe_mode branch April 9, 2024 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants