-
Notifications
You must be signed in to change notification settings - Fork 241
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
Improve documentation for QuicConnectionProtocol.transmit()
#454
Conversation
Document the fact the `transmit()` method needs to be called when sending data via the underlying `QuicConnection`.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #454 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 25 25
Lines 4881 4881
=========================================
Hits 4881 4881 ☔ View full report in Codecov by Sentry. |
@ravenblackx This one is for you |
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.
It's an improvement, but it would be nice to cross-reference transmit
from the functions that are likely to make calling it necessary (send_data
, send_headers
, maybe the datagram one). As someone trying to use the library, I didn't even notice that transmit
existed, so I wouldn't have seen documentation telling me to call it even with this update. It's surprising when a function named send
does not actually send, so making that clear in the docs would be helpful. :)
Hm, in that case it's going to mean taking a different approach. Currently, (nearly) all the methods that queue data mention you should call the I am not comfortable sprinkling references to the |
Instead of tediously copying the same text over and over, add a `aioquic_transmit` Sphinx extension to mark methods for which `datagrams_to_send` needs to be called.
66b38ea
to
de1c0f0
Compare
@ravenblackx take a look at the result here for instance: https://aioquic.readthedocs.io/en/latest/h3.html#aioquic.h3.connection.H3Connection.send_headers |
Yep, that's a lot more helpful. |
No description provided.