Retrieve bleeding edge push revision from trunk commit message.

Retrieve bleeding edge push revision from trunk commit message.

This is part of moving towards an lkgr-push script and prepares the deprecation of the prepare push commit.

BUG=
R=jkummerow@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=19482

[Michael Achenbach, 2014-02-18 10:52:59]

PTAL

[Jakob Kummerow, 2014-02-19 12:44:49]

LGTM with a bunch of comments.

https://codereview.chromium.org/169843002/diff/230001/tools/push-to-trunk/aut...
File tools/push-to-trunk/auto_roll.py (right):

https://codereview.chromium.org/169843002/diff/230001/tools/push-to-trunk/aut...
tools/push-to-trunk/auto_roll.py:108: push_pattern = "^Version
[[:digit:]]*\.[[:digit:]]*\.[[:digit:]]* (based"
Same s/*/+/ comment here... this looks like a candidate for refactoring. Let's
add a helper to class Step, roughly:

def FindLastTrunkPush():
  push_pattern = "^Version [[:digit:]]+\.[[:digit:]]+\.[[:digit:]]+ (based"
  args = "log -1 --format=%%H --grep=\"%s\" svn/trunk" % push_pattern
  return self.Git(args).strip()

https://codereview.chromium.org/169843002/diff/230001/tools/push-to-trunk/aut...
tools/push-to-trunk/auto_roll.py:113: # TODO(machebach): This metric counts all
revisions. It could be
Who's machebach?

https://codereview.chromium.org/169843002/diff/230001/tools/push-to-trunk/pus...
File tools/push-to-trunk/push_to_trunk.py (right):

https://codereview.chromium.org/169843002/diff/230001/tools/push-to-trunk/pus...
tools/push-to-trunk/push_to_trunk.py:67: options.b = None
I hope it's on your to-do list to give all these guys readable (long) names :-)

https://codereview.chromium.org/169843002/diff/230001/tools/push-to-trunk/pus...
tools/push-to-trunk/push_to_trunk.py:82: self.b = getattr(options, 'b', None)
Instead of using getattr(), the default value should be set in the OptionParser.
Again, something for the to-do list.

https://codereview.chromium.org/169843002/diff/230001/tools/push-to-trunk/pus...
tools/push-to-trunk/push_to_trunk.py:108: push_pattern = "^Version
[[:digit:]]*\.[[:digit:]]*\.[[:digit:]]* (based"
For increased correctness, s/*/+/

https://codereview.chromium.org/169843002/diff/230001/tools/push-to-trunk/pus...
tools/push-to-trunk/push_to_trunk.py:116: args = ("log -1 --format=%H %s^
--grep=\"%s\""
s/%H/%%H/ like above? Why has this worked before? I guess it's never been
executed...

https://codereview.chromium.org/169843002/diff/230001/tools/push-to-trunk/pus...
tools/push-to-trunk/push_to_trunk.py:133: args = "svn find-rev r%d" %
int(last_push_be_svn)
"%d" % int(some_string) is the complicated way of saying "%s" % some_string, eh?

https://codereview.chromium.org/169843002/diff/230001/tools/push-to-trunk/pus...
tools/push-to-trunk/push_to_trunk.py:615: "bleeding edge revision that was
pushed to trunk."))
For clarification, I'd add something like "This is used for the auto-generated
ChangeLog entry" (since the patch itself will be created between current trunk
and current bleeding_edge, as opposed to old and current bleeding_edge).

[Michael Achenbach, 2014-02-19 13:23:03]

https://codereview.chromium.org/169843002/diff/230001/tools/push-to-trunk/aut...
File tools/push-to-trunk/auto_roll.py (right):

https://codereview.chromium.org/169843002/diff/230001/tools/push-to-trunk/aut...
tools/push-to-trunk/auto_roll.py:113: # TODO(machebach): This metric counts all
revisions. It could be
On 2014/02/19 12:44:49, Jakob wrote:
> Who's machebach?

Done.

https://codereview.chromium.org/169843002/diff/230001/tools/push-to-trunk/pus...
File tools/push-to-trunk/push_to_trunk.py (right):

https://codereview.chromium.org/169843002/diff/230001/tools/push-to-trunk/pus...
tools/push-to-trunk/push_to_trunk.py:67: options.b = None
On 2014/02/19 12:44:49, Jakob wrote:
> I hope it's on your to-do list to give all these guys readable (long) names
:-)

yap

https://codereview.chromium.org/169843002/diff/230001/tools/push-to-trunk/pus...
tools/push-to-trunk/push_to_trunk.py:82: self.b = getattr(options, 'b', None)
On 2014/02/19 12:44:49, Jakob wrote:
> Instead of using getattr(), the default value should be set in the
OptionParser.
> Again, something for the to-do list.

The whole option design will get a new face in a follow up CL.

https://codereview.chromium.org/169843002/diff/230001/tools/push-to-trunk/pus...
tools/push-to-trunk/push_to_trunk.py:108: push_pattern = "^Version
[[:digit:]]*\.[[:digit:]]*\.[[:digit:]]* (based"
On 2014/02/19 12:44:49, Jakob wrote:
> For increased correctness, s/*/+/

There's no + in grep. At least on my command-line, the same command doesn't work
with s/*/+/

https://codereview.chromium.org/169843002/diff/230001/tools/push-to-trunk/pus...
tools/push-to-trunk/push_to_trunk.py:116: args = ("log -1 --format=%H %s^
--grep=\"%s\""
On 2014/02/19 12:44:49, Jakob wrote:
> s/%H/%%H/ like above? Why has this worked before? I guess it's never been
> executed...

Done.

https://codereview.chromium.org/169843002/diff/230001/tools/push-to-trunk/pus...
tools/push-to-trunk/push_to_trunk.py:133: args = "svn find-rev r%d" %
int(last_push_be_svn)
On 2014/02/19 12:44:49, Jakob wrote:
> "%d" % int(some_string) is the complicated way of saying "%s" % some_string,
eh?

Done.

https://codereview.chromium.org/169843002/diff/230001/tools/push-to-trunk/pus...
tools/push-to-trunk/push_to_trunk.py:615: "bleeding edge revision that was
pushed to trunk."))
On 2014/02/19 12:44:49, Jakob wrote:
> For clarification, I'd add something like "This is used for the auto-generated
> ChangeLog entry" (since the patch itself will be created between current trunk
> and current bleeding_edge, as opposed to old and current bleeding_edge).

Done.

[Michael Achenbach, 2014-02-19 13:24:01]

https://codereview.chromium.org/169843002/diff/230001/tools/push-to-trunk/aut...
File tools/push-to-trunk/auto_roll.py (right):

https://codereview.chromium.org/169843002/diff/230001/tools/push-to-trunk/aut...
tools/push-to-trunk/auto_roll.py:108: push_pattern = "^Version
[[:digit:]]*\.[[:digit:]]*\.[[:digit:]]* (based"
On 2014/02/19 12:44:49, Jakob wrote:
> Same s/*/+/ comment here... this looks like a candidate for refactoring. Let's
> add a helper to class Step, roughly:
> 
> def FindLastTrunkPush():
>   push_pattern = "^Version [[:digit:]]+\.[[:digit:]]+\.[[:digit:]]+ (based"
>   args = "log -1 --format=%%H --grep=\"%s\" svn/trunk" % push_pattern
>   return self.Git(args).strip()

Extract method: Done. Regarding +: see the other comment.

[Michael Achenbach, 2014-02-19 13:40:39]

Committed patchset #5 manually as r19482 (tree was closed).