-
Notifications
You must be signed in to change notification settings - Fork 14
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.
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
Add behaviour and dynamic dispatch for (de)payloaders #147
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit unsure about adding specs to callback implementation. I believe that ex_doc
will generate the spec anyway (based on the callback) and making the callback and implementation spec different is a big yucky, but on the other hand, we know that e.g. Opus.depayload
will never return {nil, depyayloader}
. I'm a bit torn when it comes to this.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #147 +/- ##
==========================================
+ Coverage 88.31% 88.51% +0.20%
==========================================
Files 36 38 +2
Lines 1891 1907 +16
==========================================
+ Hits 1670 1688 +18
+ Misses 221 219 -2
Continue to review full report in Codecov by Sentry.
|
@LVala I, too, thought that ex_doc would handle it, but alas |
Removing the Not sure which one is better, but having explicit spec seems more useful. Edit: You also need to remove |
I think that at the end of the day, we don't want to document functions from Opus/VP8 payloaders/depaylaoders. Instead, the whole documentation should be in the behaviour module or in the module that is responsible for dispatching or in both. E.g. d = ExWebRTC.RTP.Depayloader.new(ExWebRTC.RTP.Depayloader.VP8, some_options)
{frame, d} = ExWebRTC.RTP.Depayloader.depayload(d, packet) In such case, we only need to document ExWebRTC.RTP.Depayloader.VP8 module. Its functions shouldn't appear in docs. But this is the final outcome. |
@mickel8 Could you please reiterate on how exactly do you suggest the docs should look like? Specifically, which parts should I get rid of? |
Closes #143