Done.

https://codereview.chromium.org/91733003/diff/150001/tools/push-to-trunk/common_includes.py
File tools/push-to-trunk/common_includes.py (right):

https://codereview.chromium.org/91733003/diff/150001/tools/push-to-trunk/common_includes.py#newcode243
tools/push-to-trunk/common_includes.py:243: retry_on: A callback that
takes the result of the function and returns True
On 2013/11/28 13:47:27, Jakob wrote:
nit: 80col

Done.

https://codereview.chromium.org/91733003/diff/150001/tools/push-to-trunk/common_includes.py#newcode246
tools/push-to-trunk/common_includes.py:246: wait_plan: A list of waiting
delays between retries in seconds. The maximum
On 2013/11/28 13:47:27, Jakob wrote:
nit: 80col

Done.

https://codereview.chromium.org/91733003/diff/150001/tools/push-to-trunk/common_includes.py#newcode250
tools/push-to-trunk/common_includes.py:250: wait_plan = list(wait_plan
or [])
On 2013/11/28 13:47:27, Jakob wrote:
I don't think the explicit list() conversion is needed.

I'd rather keep it, since the method has side effects on the parameter.
If somebody ever saved a wait plan in a variable on the call side for
reuse...

https://codereview.chromium.org/91733003/diff/150001/tools/push-to-trunk/common_includes.py#newcode251
tools/push-to-trunk/common_includes.py:251: # Sentinel for the first
try.
On 2013/11/28 13:47:27, Jakob wrote:
s/first/last/! This is seriously misleading.

Here's a slight refactoring that doesn't need the sentinel:

wait_plan.reverse()
while True:
   try:
     result = cb()
   except Exception:
     got_exception = True
   if got_exception or retry_on(result):
     if not wait_plan:
       raise Exception("Retried too often. Giving up.")
     wait_time = wait_plan.pop()
     print "Waiting for %f seconds." % wait_time
     self._side_effect_handler.Sleep(wait_time)
     print "Retrying..."
   else:
     return result

I take it :) Just added an initialization of got_exception.

https://codereview.chromium.org/91733003/diff/150001/tools/push-to-trunk/common_includes.py#newcode286
tools/push-to-trunk/common_includes.py:286: wait_plan = wait_plan or [3,
10, 30]
On 2013/11/28 13:47:27, Jakob wrote:
I'd give the server much more time. Maybe [3, 60, 600].

Done.

https://codereview.chromium.org/91733003/diff/150001/tools/push-to-trunk/test_scripts.py
File tools/push-to-trunk/test_scripts.py (right):

https://codereview.chromium.org/91733003/diff/150001/tools/push-to-trunk/test_scripts.py#newcode272
tools/push-to-trunk/test_scripts.py:272: def Wait(seconds):
On 2013/11/28 13:47:27, Jakob wrote:
I think you mean s/Wait/Sleep/.

Done.

https://codereview.chromium.org/91733003/

--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.

Reply via email to