Adds remaining functions to cros_mark_as_stable and cleaned up error messaging.

Adds remaining functions to cros_mark_as_stable and cleaned up error messaging.

TEST=Tested with following workflow:
cros_mark_as_stable -p 'crash-reporter' -i 'boogabooga' commit
cros_mark_as_stable push
cros_mark_as_stable clean

Also passed in some real options for push_options in a branch that had nothing in it.

[sosa, 2010-07-15 04:33:22]

Intended usage from buildbot slaves:

For slaves
Adds cleanup stage before sync that runs clean command
Adds stable stage after sync that runs commit command with appropriate commit
ids and package names.
Adds push stage to all -- left TODO based on what conversations having with
bradnelson about push logic (i.e. slave logic to determine whether it should
push based on other slave changes).  I'm adding this stage mostly now so that
developers can put this in their workflow ... i.e. if they have to rev an
ebuild.

cros_mark_as_stable -p package -i commit_id commit
cros_mark_as_stable push
cros_mark_as_stable clean

-- Revs ebuild and leaves dev back on master branch in overlay repo.

[petkov, 2010-07-15 17:47:32]

Mostly style comments.

http://codereview.chromium.org/3017002/diff/4001/5001
File cros_mark_as_stable.py (right):

http://codereview.chromium.org/3017002/diff/4001/5001#newcode35
cros_mark_as_stable.py:35: 'Options to use with git-cl push using push
command.')
Indentation is off.

http://codereview.chromium.org/3017002/diff/4001/5001#newcode52
cros_mark_as_stable.py:52: 'clean' :
no whitespace before colon

http://codereview.chromium.org/3017002/diff/4001/5001#newcode86
cros_mark_as_stable.py:86: """Cleans up any uncommitted changes on either
stabilizing branch or master"""
missing period.

http://codereview.chromium.org/3017002/diff/4001/5001#newcode87
cros_mark_as_stable.py:87: _RunCommand('git reset HEAD --hard')
Are you sure that you're on stabilizing_branch when this runs? And chance that
devs can wipe out local changes by mistake, for example?

http://codereview.chromium.org/3017002/diff/4001/5001#newcode97
cros_mark_as_stable.py:97: for key in _COMMAND_DICTIONARY.keys():
Do you get them in sorted order? Maybe add sorted()

http://codereview.chromium.org/3017002/diff/4001/5001#newcode98
cros_mark_as_stable.py:98: command_usage += '  ' + key + ': ' +
_COMMAND_DICTIONARY[key] + '\n'
I think style kind of recommends using %s here -- and it might be more readable.

http://codereview.chromium.org/3017002/diff/4001/5001#newcode100
cros_mark_as_stable.py:100: # Cleanup trailing |.
Isn't something like this be a bit more clean:

commands = sorted(_COMMAND_DICTIONARY.keys())
for command in commands:
...

commands = '|'.join(commands)

http://codereview.chromium.org/3017002/diff/4001/5001#newcode101
cros_mark_as_stable.py:101: if commands.endswith('|'):  commands = commands[:-1]
remove extra space after : (if you decide to stick with this cleanup approach).

http://codereview.chromium.org/3017002/diff/4001/5001#newcode103
cros_mark_as_stable.py:103: if error_message:  error_message += '\n\n'
remove extra space after :

http://codereview.chromium.org/3017002/diff/4001/5001#newcode104
cros_mark_as_stable.py:104: generate_test_report.Die('%sUsage: %s FLAGS
[%s]\n\n%s\nFlags:%s' %
I still find it strange to print the whole use in RED. You might simply print
the Usage first and then Die with the error message at the end.

http://codereview.chromium.org/3017002/diff/4001/5001#newcode126
cros_mark_as_stable.py:126: (current_branch, _STABLE_BRANCH_NAME))
I think you need to swap the order of these two.

http://codereview.chromium.org/3017002/diff/4001/5001#newcode323
cros_mark_as_stable.py:323: if command == 'clean':
Is there any reason why Clean, Commit and Push arent' part of a StableMarker
class?

http://codereview.chromium.org/3017002/diff/4001/5001#newcode347
cros_mark_as_stable.py:347: print '''Note you will have to go into the
chromiumos-overlay directory
Per style, use """ rather than '''. Also, per style, it might be cleaner to
replace this with:

print ("blah\n"
       "blah\n")

[sosa, 2010-07-15 18:28:18]

PTAL.

http://codereview.chromium.org/3017002/diff/4001/5001
File cros_mark_as_stable.py (right):

http://codereview.chromium.org/3017002/diff/4001/5001#newcode35
cros_mark_as_stable.py:35: 'Options to use with git-cl push using push
command.')
On 2010/07/15 17:47:32, petkov wrote:
> Indentation is off.
> 

Done.

http://codereview.chromium.org/3017002/diff/4001/5001#newcode52
cros_mark_as_stable.py:52: 'clean' :
On 2010/07/15 17:47:32, petkov wrote:
> no whitespace before colon
> 

Done.

http://codereview.chromium.org/3017002/diff/4001/5001#newcode86
cros_mark_as_stable.py:86: """Cleans up any uncommitted changes on either
stabilizing branch or master"""
On 2010/07/15 17:47:32, petkov wrote:
> missing period.
> 

Done.

http://codereview.chromium.org/3017002/diff/4001/5001#newcode97
cros_mark_as_stable.py:97: for key in _COMMAND_DICTIONARY.keys():
Good suggestion.  I wasn't getting them sorted (actually getting it in reverse
order -- prob impl of dics has some sort of stack ordering) but I didn't really
mind.  Since I'm doing all this work to make this nicer to look at, should sort.

On 2010/07/15 17:47:32, petkov wrote:
> Do you get them in sorted order? Maybe add sorted()
>

http://codereview.chromium.org/3017002/diff/4001/5001#newcode98
cros_mark_as_stable.py:98: command_usage += '  ' + key + ': ' +
_COMMAND_DICTIONARY[key] + '\n'
On 2010/07/15 17:47:32, petkov wrote:
> I think style kind of recommends using %s here -- and it might be more
readable.
> 

Done.

http://codereview.chromium.org/3017002/diff/4001/5001#newcode100
cros_mark_as_stable.py:100: # Cleanup trailing |.
On 2010/07/15 17:47:32, petkov wrote:
> Isn't something like this be a bit more clean:
> 
> commands = sorted(_COMMAND_DICTIONARY.keys())
> for command in commands:
> ...
> 
> commands = '|'.join(commands)
> 

Done.

http://codereview.chromium.org/3017002/diff/4001/5001#newcode101
cros_mark_as_stable.py:101: if commands.endswith('|'):  commands = commands[:-1]
On 2010/07/15 17:47:32, petkov wrote:
> remove extra space after : (if you decide to stick with this cleanup
approach).
> 
> 

Done.

http://codereview.chromium.org/3017002/diff/4001/5001#newcode103
cros_mark_as_stable.py:103: if error_message:  error_message += '\n\n'
On 2010/07/15 17:47:32, petkov wrote:
> remove extra space after :

Done.

http://codereview.chromium.org/3017002/diff/4001/5001#newcode104
cros_mark_as_stable.py:104: generate_test_report.Die('%sUsage: %s FLAGS
[%s]\n\n%s\nFlags:%s' %
Fair enough :)

On 2010/07/15 17:47:32, petkov wrote:
> I still find it strange to print the whole use in RED. You might simply print
> the Usage first and then Die with the error message at the end.
>

http://codereview.chromium.org/3017002/diff/4001/5001#newcode126
cros_mark_as_stable.py:126: (current_branch, _STABLE_BRANCH_NAME))
On 2010/07/15 17:47:32, petkov wrote:
> I think you need to swap the order of these two.
> 

Should be on the "temp" stable branch to do the commit so I think the logic is
correct.

http://codereview.chromium.org/3017002/diff/4001/5001#newcode323
cros_mark_as_stable.py:323: if command == 'clean':
On 2010/07/15 17:47:32, petkov wrote:
> Is there any reason why Clean, Commit and Push arent' part of a StableMarker
> class?
> 

They could be but would just be static methods.  They don't have much reliance
on the class as the state does not persist across calls.

http://codereview.chromium.org/3017002/diff/4001/5001#newcode347
cros_mark_as_stable.py:347: print '''Note you will have to go into the
chromiumos-overlay directory
On 2010/07/15 17:47:32, petkov wrote:
> Per style, use """ rather than '''. Also, per style, it might be cleaner to
> replace this with:
> 
> print ("blah\n"
>        "blah\n")
> 

Done.

[petkov, 2010-07-15 18:52:24]

LGTM

[Mandeep Singh Baines, 2010-07-15 21:19:12]

LGTM