Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(61)

Issue 6720024: Plumb in subcommand options (Closed)

Created:
9 years, 11 months ago by sjg
Modified:
9 years, 10 months ago
CC:
chromium-os-reviews_chromium.org
Visibility:
Public.

Description

Plumb in subcommand options Enable passing of options to subcommands by avoiding any parsing of these at the top level. BUG=chromium-os:11785 TEST=chromite build --clean x86-generic, check it does clean python main_unittest.py (run tests) Change-Id: I4bfe86bc02d12aa207a294d143201e20875fba03 Committed: http://chrome-svn/viewvc/chromeos?view=rev&revision=f5298ef

Patch Set 1 #

Total comments: 3

Patch Set 2 : Handle '-v 3' case where option argument looks like a positional #

Patch Set 3 : Iterate until we have a command #

Total comments: 8

Patch Set 4 : Created helper function, unit tests, simplified #

Total comments: 2

Patch Set 5 : Moved docstring into docstring :-) #

Unified diffs Side-by-side diffs Delta from patch set Stats (+124 lines, -22 lines) Patch
M shell/main.py View 1 2 3 4 5 chunks +57 lines, -13 lines 0 comments Download
M shell/main_unittest.py View 1 2 3 2 chunks +62 lines, -0 lines 0 comments Download
M shell/subcmds/build_cmd.py View 2 chunks +3 lines, -5 lines 0 comments Download
M shell/subcmds/clean_cmd.py View 1 chunk +2 lines, -4 lines 0 comments Download

Messages

Total messages: 7 (0 generated)
sjg
I have plumbed in the subcommand options in this CL, just as a clean up ...
9 years, 11 months ago (2011-04-04 18:20:33 UTC) #1
diandersAtChromium
I don't think this code will work in all cases. Can you try the test ...
9 years, 11 months ago (2011-04-04 20:42:48 UTC) #2
sjg
PTAL I may have a simpler solution.
9 years, 11 months ago (2011-04-04 22:31:58 UTC) #3
diandersAtChromium
I think it can be a little cleaner still... http://codereview.chromium.org/6720024/diff/5001/shell/main.py File shell/main.py (right): http://codereview.chromium.org/6720024/diff/5001/shell/main.py#newcode142 shell/main.py:142: ...
9 years, 11 months ago (2011-04-08 00:43:46 UTC) #4
sjg
Thanks Doug, PTAL http://codereview.chromium.org/6720024/diff/5001/shell/main.py File shell/main.py (right): http://codereview.chromium.org/6720024/diff/5001/shell/main.py#newcode142 shell/main.py:142: # We want to parse only ...
9 years, 11 months ago (2011-04-08 20:25:26 UTC) #5
diandersAtChromium
LGTM w/ last nit. Ooooo. Shiny! http://codereview.chromium.org/6720024/diff/9001/shell/main.py File shell/main.py (right): http://codereview.chromium.org/6720024/diff/9001/shell/main.py#newcode96 shell/main.py:96: # Helper function ...
9 years, 11 months ago (2011-04-08 20:35:41 UTC) #6
sjg
9 years, 11 months ago (2011-04-08 21:00:19 UTC) #7
http://codereview.chromium.org/6720024/diff/9001/shell/main.py
File shell/main.py (right):

http://codereview.chromium.org/6720024/diff/9001/shell/main.py#newcode96
shell/main.py:96: # Helper function to parse arguments and split into one we
understand, and
On 2011/04/08 20:35:41, diandersAtChromium wrote:
> Last nit: can you make this a docstring instead of a header comment?

doh.

Powered by Google App Engine
This is Rietveld 408576698