Issue 13122043: Fix FLAGS usage in args

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

Issue 13122043: Fix FLAGS usage in args

Can't Edit
Can't Publish+Mail
Start Review

Created:
10 years, 8 months ago by ekasper

Modified:
10 years, 8 months ago

Reviewers:
Eran

CC:
ctlog-opensource-review_google.com

Visibility:
Public.

More Reviews

Patch Set 2 : remove client recreation #

Created: 10 years, 8 months ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+18 lines, -2 lines)			Patch
	M	src/python/ct/client/log_client.py	View		2 chunks	+2 lines, -1 line	0 comments	Download
	M	src/python/ct/client/log_client_test.py	View	1	1 chunk	+16 lines, -1 line	0 comments	Download

Messages

Total messages: 5

Expand All Messages | Collapse All Messages

ekasper

Passing a FLAGS value in arguments will always bind to the default flag value rather ...

10 years, 8 months ago (2013-08-20 12:43:10 UTC) #1

Eran

The current way (passing in the size from the flag as default) makes more sense. ...

10 years, 8 months ago (2013-08-20 12:45:35 UTC) #2

ekasper

On 2013/08/20 12:45:35, Eran wrote: > The current way (passing in the size from the ...

10 years, 8 months ago (2013-08-20 12:50:46 UTC) #3

Eran

LGTM. https://codereview.appspot.com/13122043/diff/1/src/python/ct/client/log_client_test.py File src/python/ct/client/log_client_test.py (right): https://codereview.appspot.com/13122043/diff/1/src/python/ct/client/log_client_test.py#newcode165 src/python/ct/client/log_client_test.py:165: mock_responder.reset_mock() IMHO a separate test is preferable, since ...

10 years, 8 months ago (2013-08-20 12:52:47 UTC) #4

ekasper

10 years, 8 months ago (2013-08-20 12:55:00 UTC) #5

https://codereview.appspot.com/13122043/diff/1/src/python/ct/client/log_clien...
File src/python/ct/client/log_client_test.py (right):

https://codereview.appspot.com/13122043/diff/1/src/python/ct/client/log_clien...
src/python/ct/client/log_client_test.py:165: mock_responder.reset_mock()
On 2013/08/20 12:52:48, Eran wrote:
> IMHO a separate test is preferable, since you're re-creating the client,
rather
> than do additional asserts on the same client.

Good copy-paste catch. I don't have to recreate the client; resetting call
attributes on the mock is sufficient. So I've removed that instead.

Expand All Messages | Collapse All Messages