On 2015/03/27 11:36:40, Hablich wrote:
https://codereview.chromium.org/1033043002/diff/1/tools/release/mergeinfo.py
File tools/release/mergeinfo.py (right):
https://codereview.chromium.org/1033043002/diff/1/tools/release/mergeinfo.py#newcode2
tools/release/mergeinfo.py:2: # Copyright 2013 the V8 project authors. All
rights reserved.
On 2015/03/26 19:31:00, Michael Achenbach (travelling) wrote:
> Please use short license header, see newer files, e.g. create_release.py
with
> 2015.
Done.
https://codereview.chromium.org/1033043002/diff/1/tools/release/mergeinfo.py#newcode35
tools/release/mergeinfo.py:35: def printAnalysis(gitWorkingDir,
hashToSearch):
On 2015/03/26 19:31:00, Michael Achenbach (travelling) wrote:
> nit: as it is a new file rather use pythonish method names, e.g.:
> print_analysis and git_execute
Acknowledged.
https://codereview.chromium.org/1033043002/diff/1/tools/release/mergeinfo.py#newcode37
tools/release/mergeinfo.py:37: gitExecute(gitWorkingDir, ['status'])
On 2015/03/26 19:31:00, Michael Achenbach (travelling) wrote:
> I'd make the api of this tool more concise. Just search for the
cherry-pick
> commits and nothing else - e.g. no show and no status, etc. Then it's
easier
to
> use the tool from other tools and consume the output.
Not meant for consumption by other tools. For now it should show you every
information it can find on a hash. And reverts are also very interesting.
https://codereview.chromium.org/1033043002/diff/1/tools/release/mergeinfo.py#newcode39
tools/release/mergeinfo.py:39: gitExecute(gitWorkingDir, ['pull'])
On 2015/03/26 19:31:00, Michael Achenbach (travelling) wrote:
> I wouldn't let the script attempt to pull as pull merges to the current
branch.
> Rather use fetch. Or even better, assume the checkout is in an
up-to-date
state.
Acknowledged.
https://codereview.chromium.org/1033043002/diff/1/tools/release/mergeinfo.py#newcode45
tools/release/mergeinfo.py:45: gitExecute(gitWorkingDir, ["log",'--all',
'--grep='+hashToSearch])
On 2015/03/26 19:31:00, Michael Achenbach (travelling) wrote:
> Add --format="%H" so that only the hashes are returned.
>
> Grepping for the hash will lead to some false positives e.g. commits
that
revert
> the original commit and mention it in the commit message. Rather search
for
the
> exact expression "Merged deadbeef"
>
> log searches only on the current HEAD IIRC. I think you need something
like
> rev-list - not sure right now. rev-list only returns hashes which you
could
then
> use in another command. Or use log on all the heads and branch-heads.
It should be human readable. I like it more verbose. The parameter --all
searches also the refs.
https://codereview.chromium.org/1033043002/diff/1/tools/release/mergeinfo.py#newcode54
tools/release/mergeinfo.py:54: parser.add_argument("-g", "--git-dir",
required=True,
On 2015/03/26 19:31:00, Michael Achenbach (travelling) wrote:
> This should be optional and just work for the current git checkout in
which
the
> script lives.
Acknowledged.
https://codereview.chromium.org/1033043002/diff/1/tools/release/mergeinfo.py#newcode57
tools/release/mergeinfo.py:57: parser.add_argument("--hash", help="Hash
of the
commit to be searched.", required=True)
On 2015/03/26 19:31:00, Michael Achenbach (travelling) wrote:
> I'd make this a parameterless argument so that the tool can be called:
> mergeinfo.py deadbeef
Acknowledged.
https://codereview.chromium.org/1033043002/diff/20001/tools/release/mergeinfo.py
File tools/release/mergeinfo.py (right):
https://codereview.chromium.org/1033043002/diff/20001/tools/release/mergeinfo.py#newcode32
tools/release/mergeinfo.py:32: parser.add_argument('hash',nargs=1,
help="Hash
of
the commit to be searched.")
On 2015/03/27 10:55:01, Michael Achenbach (travelling) wrote:
> nit: space after comma
Done.
My fault. Forgot to publish the comments.
https://codereview.chromium.org/1033043002/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
