elixir-webrtc / ex_webrtc

An Elixir implementation of the W3C WebRTC API
Apache License 2.0
270 stars 8 forks source link

Add Transceiver guide #56

Closed mickel8 closed 5 months ago

codecov[bot] commented 5 months ago

Codecov Report

Merging #56 (b985785) into master (8bf58fa) will not change coverage. Report is 1 commits behind head on master. The diff coverage is n/a.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #56 +/- ## ======================================= Coverage 89.02% 89.02% ======================================= Files 25 25 Lines 1057 1057 ======================================= Hits 941 941 Misses 116 116 ``` ------ [Continue to review full report in Codecov by Sentry](https://app.codecov.io/gh/elixir-webrtc/ex_webrtc/pull/56?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=elixir-webrtc). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=elixir-webrtc) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://app.codecov.io/gh/elixir-webrtc/ex_webrtc/pull/56?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=elixir-webrtc). Last update [8bf58fa...b985785](https://app.codecov.io/gh/elixir-webrtc/ex_webrtc/pull/56?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=elixir-webrtc). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=elixir-webrtc).
mickel8 commented 5 months ago

Some general remarks:

use Grammarly or paste the text into ChatGPT and tell it to fix the grammar and punctuation,

done

like I mentioned somewhere, I feel like the guide is missing a glossary, as a lot of the terminology might be obvious for us but not necessarily for someone who just used the JS API and didn't really look into SDP and stuff. I would also move the explanations of transceiver properties (direction etc.) there,

the guide goes quite in-depth from the very beginning, I would love to see some introductory example (maybe showing differences between just using addTrack and using transceivers directly)

maybe it would be nice to crate a section where all of the API functions used later on are explained (focusing on intuition), what do you think of adding a nice diagram showing relations between transceivers/senders/transports etc. with the new ex_doc diagram feature?

Great point! However, this is quite a lot of work and the idea was to have more advanced guide for people who more or less know what webrtc is and are looking for deep understanding of transceivers and what they can do with them.

I think that all of the things you listed i.e. glossary, more basic examples, diagrams, API explanation is for another section(s) and for another task.

For now I would favor renaming the guide to Mastering Transceivers or something like this and stating at the beginning what's the purpose of this guide.

link to some introductory guide to WebRTC API in the beginning of the guide?

:+1:

LVala commented 5 months ago

Great point! However, this is quite a lot of work and the idea was to have more advanced guide for people who more or less know what webrtc is and are looking for deep understanding of transceivers and what they can do with them.

I think that all of the things you listed i.e. glossary, more basic examples, diagrams, API explanation is for another section(s) and for another task.

For now I would favor renaming the guide to Mastering Transceivers or something like this and stating at the beginning what's the purpose of this guide.

Sure