-
Notifications
You must be signed in to change notification settings - Fork 423
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
UCT/DOC: document uct_ep_disconnect return codes #4586
UCT/DOC: document uct_ep_disconnect return codes #4586
Conversation
@tonycurtis @bbenton - please take a look. Overall, looks good to me. |
Mellanox CI: PASSED on 25 workers (click for details)Note: the logs will be deleted after 23-Dec-2019
|
Mellanox CI: PASSED on 25 workers (click for details)Note: the logs will be deleted after 23-Dec-2019
|
@evgeny-leksikov is the concept of locally and remotely connected ep clear to a user..? |
There are mentions of remote peer/side in the func description, also I tried to explain it referring to corresponding callbacks, do you think need to add something else? |
There are mentions of remote peer but not of a remote connected ep. IMHO it would be better to explain. |
@yosefe WDTY? IMO it's clear from whole the description
I don't think so because such calls don't have side effect except notification to user. |
@alinask made valid point. Probably you can state that |
I'm sure @tonycurtis can come up with more compelling clarification :) |
On Dec 17, 2019, at 9:40 AM, Pavel Shamis (Pasha) ***@***.***> wrote:
@alinask <https://github.com/alinask> made valid point. Probably you can state that the disconnect request was initiated, but the remote peer didn't respond on the request and callback registered under XXX has not been invoked to process the request
So, how about:
The disconnect request has been initiated, but the remote peer has not (yet?) responded to this request, and consequently the registered callback has not been invoked to handle/process the request.
?
Also a query whether the @ref should be to the type names?
Tony
|
@tonycurtis thank you!
I think this is pretty useful because goxygen generates valid link to the type thich is documented how to use it in correct way. |
Mellanox CI: PASSED on 25 workers (click for details)Note: the logs will be deleted after 26-Dec-2019
|
@tonycurtis Can you please review? |
I'm 👍 |
Looks ok (have been traveling, mostly incommunicado)
tony
|
What
Document
uct_ep_disconnect
return codesWhy ?
To make clearer specially handled errors