The major change is refactoring to make functions more accessible for other modules.

The major change is refactoring to make functions more accessible for other modules.

Besides changing a lot of _ to not _, I added lots of docstrings, moved things around,
factored out the Marking component of the EBuildStableMarker (for use with other CL),
and fixed the version logic.

The versioning logic was too hacktastic.  So I took a note from David's changes to 
take the information from pkgsplit rather than doing the ugly parsing I was doing before.
This is much more robust in the case of no _r*'s and _rc or _alpha*, etc.

This CL also adds the ability to test Push changes in 2 ways.  One using --dryrun which allows someone to call it by not actually push to the remote server and two, a unit test for the Push method.

Used in http://codereview.chromium.org/4798001/

BUG=chromiumos:8693
TEST=Ran unit tests and chrome mark as stable.  Will run some more tests.

Committed: http://chrome-svn/viewvc/chromeos?view=rev&revision=be485ce

Committed: http://chrome-svn/viewvc/chromeos?view=rev&revision=3e68973

[davidjames, 2010-11-18 21:56:32]

LGTM w/nit

http://codereview.chromium.org/5172003/diff/2001/cros_mark_as_stable.py
File cros_mark_as_stable.py (right):

http://codereview.chromium.org/5172003/diff/2001/cros_mark_as_stable.py#newco...
cros_mark_as_stable.py:415: 
Extra newline.

[sosa, 2010-11-19 00:26:22]

PTAL.  Fixes for the push issues are here.  You can see these in diffs 3 & 4.

[sosa, 2010-11-19 00:36:05]

PTAL added a dryrun flag

[sosa, 2010-11-19 00:43:44]

PTAL, actually call git push --dry-run rather than not at all

[davidjames, 2010-11-19 00:58:41]

Only got some nits here. Could you add a unit test for pushing, since that's the
part where we had problems with last time we pushed?

http://codereview.chromium.org/5172003/diff/19001/cros_mark_as_stable.py
File cros_mark_as_stable.py (right):

http://codereview.chromium.org/5172003/diff/19001/cros_mark_as_stable.py#newc...
cros_mark_as_stable.py:248: description = 'Marking set of ebuilds as
stable\n\n%s' % description
Could you log the description here? Helpful for debugging in case we get a crazy
description.

[sosa, 2010-11-19 01:20:49]

PTAL Fixed nit and added unit test

[davidjames, 2010-11-19 01:24:06]

Could you update the change description above to indicate the tests you
mentioned over chat?

LGTM. Please watch a full preflight buildbot run go through after committing.

[sosa, 2010-11-19 01:30:51]

Sure thing.  Thanks for the review.

[davidjames, 2010-11-19 18:17:27]

http://codereview.chromium.org/5172003/diff/35001/cros_mark_as_stable.py
File cros_mark_as_stable.py (right):

http://codereview.chromium.org/5172003/diff/35001/cros_mark_as_stable.py#newc...
cros_mark_as_stable.py:34: 'Options to use with git-cl push using push
command.')
Thanks! I know this is already submitted but I have two suggestions that we can
address later:
 1) Somehow I didn't notice that you had deleted this option. For future
changes, could you mention all changes you make in the log message? That way
I'll be able to review it.
 2) Could you update --debug mode for cbuildbot.py to actually call
cros_mark_as_stable.py push with the --dryrun option? That way, we'll get more
testing for the options supplied to push.
  3) Once you do (2) it'd be fine to remove push_options in both places
(cbuildbot.py and cros_mark_as_stable.py)

[sosa, 2010-11-19 18:34:24]

I already am getting ready a CL for 2 and 3.  I'll work on 1, sorry about that.

On Fri, Nov 19, 2010 at 10:17 AM,  <davidjames@chromium.org> wrote:
>
> http://codereview.chromium.org/5172003/diff/35001/cros_mark_as_stable.py
> File cros_mark_as_stable.py (right):
>
>
http://codereview.chromium.org/5172003/diff/35001/cros_mark_as_stable.py#newc...
> cros_mark_as_stable.py:34: 'Options to use with git-cl push using push
> command.')
> Thanks! I know this is already submitted but I have two suggestions that
> we can address later:
>  1) Somehow I didn't notice that you had deleted this option. For future
> changes, could you mention all changes you make in the log message? That
> way I'll be able to review it.
>  2) Could you update --debug mode for cbuildbot.py to actually call
> cros_mark_as_stable.py push with the --dryrun option? That way, we'll
> get more testing for the options supplied to push.
>  3) Once you do (2) it'd be fine to remove push_options in both places
> (cbuildbot.py and cros_mark_as_stable.py)
>
> http://codereview.chromium.org/5172003/
>