Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(10)

Issue 7751044: Queues (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years ago by A. Jesse Jiryu Davis
Modified:
4 years ago
Reviewers:
Nikolay Kim, gvrpython, GvR
Visibility:
Public.

Description

Queues

Patch Set 1 #

Total comments: 24

Patch Set 2 : Responses to critique #

Total comments: 25

Patch Set 3 : Response to 2nd round of critique #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+292 lines, -406 lines) Patch
M tests/queues_test.py View 1 2 11 chunks +155 lines, -172 lines 2 comments Download
M tulip/queues.py View 1 2 8 chunks +137 lines, -234 lines 2 comments Download

Messages

Total messages: 21
A. Jesse Jiryu Davis
For Issue 7: http://code.google.com/p/tulip/issues/detail?id=7 From my initial proposal: Tulip already has Lock, EventWaiter, Condition, and ...
4 years ago (2013-03-18 18:35:49 UTC) #1
Nikolay Kim
On 2013/03/18 18:35:49, A. Jesse Jiryu Davis wrote: > For Issue 7: http://code.google.com/p/tulip/issues/detail?id=7 > > ...
4 years ago (2013-03-18 18:58:43 UTC) #2
GvR
This is not a full review, just a bunch of notes on style and a ...
4 years ago (2013-03-18 20:10:22 UTC) #3
A. Jesse Jiryu Davis
On 2013/03/18 20:10:22, GvR wrote: > This is not a full review, just a bunch ...
4 years ago (2013-03-18 21:53:40 UTC) #4
A. Jesse Jiryu Davis
Responses to Guido are inline. Nikolay: > 1. Should we reuse locks? I tried that ...
4 years ago (2013-03-19 18:28:14 UTC) #5
A. Jesse Jiryu Davis
Second patch uploaded. This has: * Nikolay's idea to separate blocking and nonblocking methods * ...
4 years ago (2013-03-19 18:35:39 UTC) #6
GvR
Good revision! Did you sign the PSF contrib agreement yet? I'm still pushing back on ...
4 years ago (2013-03-19 20:55:15 UTC) #7
Nikolay Kim
i think _Waiter is quite useful on its own. lets rename it to something like ...
4 years ago (2013-03-19 21:56:45 UTC) #8
gvrpython
On Tue, Mar 19, 2013 at 2:56 PM, <fafhrd91@gmail.com> wrote: > i think _Waiter is ...
4 years ago (2013-03-19 22:04:45 UTC) #9
Nikolay Kim
On 2013/03/19 22:04:45, gvrpython wrote: > On Tue, Mar 19, 2013 at 2:56 PM, <mailto:fafhrd91@gmail.com> ...
4 years ago (2013-03-19 22:24:36 UTC) #10
GvR
On 2013/03/19 22:24:36, Nikolay Kim wrote: > On 2013/03/19 22:04:45, gvrpython wrote: > > On ...
4 years ago (2013-03-19 22:29:42 UTC) #11
Nikolay Kim
i've just added timeout support to Future.
4 years ago (2013-03-20 18:28:38 UTC) #12
gvrpython
(So, what Nikolay says, you can hg pull -u your repo, and then get rid ...
4 years ago (2013-03-20 18:46:37 UTC) #13
Nikolay Kim
> https://codereview.appspot.com/7751044/diff/8001/tulip/queues.py#newcode207 > tulip/queues.py:207: yield > Hm... This use of a bare yield is actually ...
4 years ago (2013-03-20 22:11:29 UTC) #14
A. Jesse Jiryu Davis
https://codereview.appspot.com/7751044/diff/1/tulip/queues.py File tulip/queues.py (right): https://codereview.appspot.com/7751044/diff/1/tulip/queues.py#newcode159 tulip/queues.py:159: def set_maxsize(self, maxsize): On 2013/03/19 20:55:15, GvR wrote: > ...
4 years ago (2013-03-20 22:23:46 UTC) #15
A. Jesse Jiryu Davis
This patch might be ready to merge (no outstanding issues I know of). * _Waiter ...
4 years ago (2013-03-20 22:27:00 UTC) #16
Nikolay Kim
On 2013/03/20 22:27:00, A. Jesse Jiryu Davis wrote: > This patch might be ready to ...
4 years ago (2013-03-20 22:59:20 UTC) #17
GvR
Just one thing left, really -- maxsize needs to be a property. Feel free to ...
4 years ago (2013-03-21 00:16:48 UTC) #18
A. Jesse Jiryu Davis
https://codereview.appspot.com/7751044/diff/23001/tests/queues_test.py File tests/queues_test.py (right): https://codereview.appspot.com/7751044/diff/23001/tests/queues_test.py#newcode126 tests/queues_test.py:126: @tasks.task On 2013/03/21 00:16:48, GvR wrote: > You can ...
4 years ago (2013-03-21 00:50:26 UTC) #19
A. Jesse Jiryu Davis
On 2013/03/21 00:50:26, A. Jesse Jiryu Davis wrote: > https://codereview.appspot.com/7751044/diff/23001/tests/queues_test.py > File tests/queues_test.py (right): > ...
4 years ago (2013-03-21 01:02:08 UTC) #20
GvR
4 years ago (2013-03-21 01:46:23 UTC) #21
Message was sent while issue was closed.
On 2013/03/21 01:02:08, A. Jesse Jiryu Davis wrote:
> Thanks Guido and Nikolay, this has been an insanely great code review.

Thanks Jesse for the contribution!!  I hope you will make more.

(Did you get the email about using parameter-less super()?)
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 80a51fa-tainted