Keyboard Shortcuts

File
u :up to issue
m :publish + mail comments
M :edit review message
j / k :jump to file after / before current file
J / K :jump to next file with a comment after / before current file
Side-by-side diff
i :toggle intra-line diffs
e :expand all comments
c :collapse all comments
s :toggle showing all comments
n / p :next / previous diff chunk or comment
N / P :next / previous comment
<Up> / <Down> :next / previous line
<Enter> :respond to / edit current comment
d :mark current comment as done
Issue
u :up to list of issues
m :publish + mail comments
j / k :jump to patch after / before current patch
o / <Enter> :open current patch in side-by-side view
i :open current patch in unified diff view
Issue List
j / k :jump to issue after / before current issue
o / <Enter> :open current issue
# : close issue
Comment/message editing
<Ctrl> + s or <Ctrl> + Enter :save comment
<Esc> :cancel edit
Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(263)
Issues Repositories Search
Open Issues | Closed Issues | All Issues | Sign in with your Google Account to create issues and add comments

Issue 66430046: docs refactoring

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 10 months ago by yselivanov
Modified:
11 years, 10 months ago
Reviewers:
GvR
Visibility:
Public.
docs refactoring

Patch Set 1 #

Total comments: 8

Patch Set 2 : v2 #

Created: 11 years, 10 months ago
Download [raw] [tar.bz2]
Unified diffs Side-by-side diffs Delta from patch set Stats (+49 lines, -38 lines) Patch
M Doc/library/asyncio-eventloop.rst View 1 17 chunks +23 lines, -22 lines 0 comments Download
M Doc/library/asyncio-stream.rst View 7 chunks +7 lines, -7 lines 0 comments Download
M Doc/library/asyncio-sync.rst View 7 chunks +7 lines, -7 lines 0 comments Download
M Doc/library/asyncio-task.rst View 1 4 chunks +12 lines, -2 lines 0 comments Download
Total messages: 7
|
GvR
https://codereview.appspot.com/66430046/diff/1/Doc/library/asyncio-eventloop.rst File Doc/library/asyncio-eventloop.rst (right): https://codereview.appspot.com/66430046/diff/1/Doc/library/asyncio-eventloop.rst#newcode391 Doc/library/asyncio-eventloop.rst:391: which result is set to ``None`` on success. which ...
11 years, 10 months ago (2014年02月20日 19:53:31 UTC) #1
https://codereview.appspot.com/66430046/diff/1/Doc/library/asyncio-eventloop.rst
File Doc/library/asyncio-eventloop.rst (right):
https://codereview.appspot.com/66430046/diff/1/Doc/library/asyncio-eventloop....
Doc/library/asyncio-eventloop.rst:391: which result is set to ``None`` on
success.
which -> whose
Also no comma needed on the previous line.
https://codereview.appspot.com/66430046/diff/1/Doc/library/asyncio-eventloop....
Doc/library/asyncio-eventloop.rst:394: much data, if any, was successfully sent.
This is correct, but I expect that people will believe it to be a bug and try to
"fix" it. That's of course impossible because giving the data to the kernel
doesn't mean the other side received it -- you'd need an end to end conformation
before you can know that it was truly received. I wonder if we can add a note
clarifying this here by rephrasing it as "how much data, if any, was
successfully processed by the receiving end of the connection."
https://codereview.appspot.com/66430046/diff/1/Doc/library/asyncio-eventloop....
Doc/library/asyncio-eventloop.rst:441: returns a :class:`Future`.
Hmm... Can we under-specify this and state that it can return either a future or
a coroutine? Actually I would like to do that for (almost) *any* defined method
or function in the API that is a coroutine or returns a Future -- we really
should warn our users that this aspect of the spec is fungible. Or perhaps we
can document this once in the definition to which "coroutine" links, and then
(again, with a few obvious exceptions, e.g. async() itself :-) document
everything as "a coroutine" even if the implementation currently returns a
Future?
Sign in to reply to this message.
yselivanov
https://codereview.appspot.com/66430046/diff/1/Doc/library/asyncio-eventloop.rst File Doc/library/asyncio-eventloop.rst (right): https://codereview.appspot.com/66430046/diff/1/Doc/library/asyncio-eventloop.rst#newcode391 Doc/library/asyncio-eventloop.rst:391: which result is set to ``None`` on success. On ...
11 years, 10 months ago (2014年02月20日 20:32:14 UTC) #2
https://codereview.appspot.com/66430046/diff/1/Doc/library/asyncio-eventloop.rst
File Doc/library/asyncio-eventloop.rst (right):
https://codereview.appspot.com/66430046/diff/1/Doc/library/asyncio-eventloop....
Doc/library/asyncio-eventloop.rst:391: which result is set to ``None`` on
success.
On 2014年02月20日 19:53:31, GvR wrote:
> which -> whose
> 
> Also no comma needed on the previous line.
OK
https://codereview.appspot.com/66430046/diff/1/Doc/library/asyncio-eventloop....
Doc/library/asyncio-eventloop.rst:394: much data, if any, was successfully sent.
On 2014年02月20日 19:53:31, GvR wrote:
> This is correct, but I expect that people will believe it to be a bug and try
to
> "fix" it. That's of course impossible because giving the data to the kernel
> doesn't mean the other side received it -- you'd need an end to end
conformation
> before you can know that it was truly received. I wonder if we can add a note
> clarifying this here by rephrasing it as "how much data, if any, was
> successfully processed by the receiving end of the connection."
OK, much better with this change
https://codereview.appspot.com/66430046/diff/1/Doc/library/asyncio-eventloop....
Doc/library/asyncio-eventloop.rst:441: returns a :class:`Future`.
On 2014年02月20日 19:53:31, GvR wrote:
> Hmm... Can we under-specify this and state that it can return either a future
or
> a coroutine? Actually I would like to do that for (almost) *any* defined
method
> or function in the API that is a coroutine or returns a Future -- we really
> should warn our users that this aspect of the spec is fungible.
But what if someone prefer to use asyncio without coroutines? Strictly callback
approach. They may find 'getaddrinfo()' function useful, and start using its
result as a Future, i.e. add/remove callbacks on it etc.
> Or perhaps we
> can document this once in the definition to which "coroutine" links, and then
> (again, with a few obvious exceptions, e.g. async() itself :-) document
> everything as "a coroutine" even if the implementation currently returns a
> Future?
That could work. But again, people will look in the source code, and if they
see that a future is returned, they might start using its APIs. And even with
this remark in docs, it will be hard for us to change return value from Future
to coroutine, or vice versa.
Sign in to reply to this message.
GvR
https://codereview.appspot.com/66430046/diff/1/Doc/library/asyncio-eventloop.rst File Doc/library/asyncio-eventloop.rst (right): https://codereview.appspot.com/66430046/diff/1/Doc/library/asyncio-eventloop.rst#newcode441 Doc/library/asyncio-eventloop.rst:441: returns a :class:`Future`. On 2014年02月20日 20:32:14, yselivanov wrote: > ...
11 years, 10 months ago (2014年02月20日 20:37:54 UTC) #3
https://codereview.appspot.com/66430046/diff/1/Doc/library/asyncio-eventloop.rst
File Doc/library/asyncio-eventloop.rst (right):
https://codereview.appspot.com/66430046/diff/1/Doc/library/asyncio-eventloop....
Doc/library/asyncio-eventloop.rst:441: returns a :class:`Future`.
On 2014年02月20日 20:32:14, yselivanov wrote:
> On 2014年02月20日 19:53:31, GvR wrote:
> > Hmm... Can we under-specify this and state that it can return either a
future
> or
> > a coroutine? Actually I would like to do that for (almost) *any* defined
> method
> > or function in the API that is a coroutine or returns a Future -- we really
> > should warn our users that this aspect of the spec is fungible.
> 
> But what if someone prefer to use asyncio without coroutines? Strictly
callback
> approach. They may find 'getaddrinfo()' function useful, and start using its
> result as a Future, i.e. add/remove callbacks on it etc.
That's what async() is for.
> > Or perhaps we
> > can document this once in the definition to which "coroutine" links, and
then
> > (again, with a few obvious exceptions, e.g. async() itself :-) document
> > everything as "a coroutine" even if the implementation currently returns a
> > Future?
> 
> That could work. But again, people will look in the source code, and if they
> see that a future is returned, they might start using its APIs. And even with
> this remark in docs, it will be hard for us to change return value from Future
> to coroutine, or vice versa.
I went back and forth a few times for various functions/methods during
development and it was mostly a non-event each time. This is also why I prefer
to document (almost) everything as "a coroutine", since that's the more
conservative assumption.
Sign in to reply to this message.
yselivanov
https://codereview.appspot.com/66430046/diff/1/Doc/library/asyncio-eventloop.rst File Doc/library/asyncio-eventloop.rst (right): https://codereview.appspot.com/66430046/diff/1/Doc/library/asyncio-eventloop.rst#newcode441 Doc/library/asyncio-eventloop.rst:441: returns a :class:`Future`. > I went back and forth ...
11 years, 10 months ago (2014年02月20日 20:46:45 UTC) #4
https://codereview.appspot.com/66430046/diff/1/Doc/library/asyncio-eventloop.rst
File Doc/library/asyncio-eventloop.rst (right):
https://codereview.appspot.com/66430046/diff/1/Doc/library/asyncio-eventloop....
Doc/library/asyncio-eventloop.rst:441: returns a :class:`Future`.
> I went back and forth a few times for various functions/methods during
> development and it was mostly a non-event each time. This is also why I prefer
> to document (almost) everything as "a coroutine", since that's the more
> conservative assumption.
So what do you suggest?
If a method/function is decorated with a @coroutine decorator, we
write "The method is a coroutine"
If a method/function returns a Future, we write
"The method returns a coroutine object"
?
Sign in to reply to this message.
GvR
On 2014年02月20日 20:46:45, yselivanov wrote: > https://codereview.appspot.com/66430046/diff/1/Doc/library/asyncio-eventloop.rst > File Doc/library/asyncio-eventloop.rst (right): > > https://codereview.appspot.com/66430046/diff/1/Doc/library/asyncio-eventloop.rst#newcode441 > ...
11 years, 10 months ago (2014年02月20日 20:50:38 UTC) #5
On 2014年02月20日 20:46:45, yselivanov wrote:
>
https://codereview.appspot.com/66430046/diff/1/Doc/library/asyncio-eventloop.rst
> File Doc/library/asyncio-eventloop.rst (right):
> 
>
https://codereview.appspot.com/66430046/diff/1/Doc/library/asyncio-eventloop....
> Doc/library/asyncio-eventloop.rst:441: returns a :class:`Future`.
> > I went back and forth a few times for various functions/methods during
> > development and it was mostly a non-event each time. This is also why I
prefer
> > to document (almost) everything as "a coroutine", since that's the more
> > conservative assumption.
> 
> 
> So what do you suggest?
> 
> If a method/function is decorated with a @coroutine decorator, we
> write "The method is a coroutine"
> 
> If a method/function returns a Future, we write
> "The method returns a coroutine object"
> 
> ?
I suggest always writing "it is a coroutine" and stretching the meaning of
coroutine.
Sign in to reply to this message.
yselivanov
> I suggest always writing "it is a coroutine" and stretching the meaning of > ...
11 years, 10 months ago (2014年02月20日 21:08:48 UTC) #6
> I suggest always writing "it is a coroutine" and stretching the meaning of
> coroutine.
OK, please review the new version.
Sign in to reply to this message.
GvR
LGTM. Poor Larry!
11 years, 10 months ago (2014年02月20日 21:16:42 UTC) #7
LGTM. Poor Larry!
Sign in to reply to this message.
|
Powered by Google App Engine
This is Rietveld f62528b

AltStyle によって変換されたページ (->オリジナル) /