Some more readability comments and replies, before diving into the
functionality
a bit deeper.
Could you add an example call of the script in the CL description? We
sometimes
add this:
TEST=tools/release/search_related_commits.py ...
https://codereview.chromium.org/1098123002/diff/80001/tools/release/search_related_commits.py
File tools/release/search_related_commits.py (right):
https://codereview.chromium.org/1098123002/diff/80001/tools/release/search_related_commits.py#newcode30
tools/release/search_related_commits.py:30: if len(related_commits) > 0:
On 2015/04/27 11:06:08, Hablich wrote:
On 2015/04/24 12:45:12, Michael Achenbach wrote:
> nit:
> if related_commits:
I like it that way as it is more explicit and better readable.
Hmm, but not pythonish. Most places in the infrastructure code, we use
the shorter version as
bool(related_commits) implies len(related_commits) > 0
https://codereview.chromium.org/1098123002/diff/80001/tools/release/search_related_commits.py#newcode60
tools/release/search_related_commits.py:60: found_by_hash =
(_git_execute(
I don't know for what the outer parentheses are needed? Also in the
remainder of this file...
https://codereview.chromium.org/1098123002/diff/80001/tools/release/search_related_commits.py#newcode162
tools/release/search_related_commits.py:162:
(_git_execute(options.git_dir,
On 2015/04/24 12:45:12, Michael Achenbach wrote:
nit: too many parenthesis
What about this? Maybe also move the lambda into a local variable for
better readability.
https://codereview.chromium.org/1098123002/diff/80001/tools/release/search_related_commits.py#newcode168
tools/release/search_related_commits.py:168: print "+" + (_git_execute(
On 2015/04/27 11:06:07, Hablich wrote:
On 2015/04/24 12:45:12, Michael Achenbach wrote:
> Difficult to read the way the parenthesis are set and the way it is
indented.
> Maybe extract method?
Done.
Much more readable :)
https://codereview.chromium.org/1098123002/diff/100001/tools/release/search_related_commits.py
File tools/release/search_related_commits.py (right):
https://codereview.chromium.org/1098123002/diff/100001/tools/release/search_related_commits.py#newcode16
tools/release/search_related_commits.py:16: hash = of
not liked
https://codereview.chromium.org/1098123002/diff/100001/tools/release/search_related_commits.py#newcode43
tools/release/search_related_commits.py:43: hash = of
not liked
https://codereview.chromium.org/1098123002/diff/100001/tools/release/search_related_commits.py#newcode67
tools/release/search_related_commits.py:67: git_working_dir, (
You don't need additional parentheses in python as long as you are in
the scope of one opening parenthesis.
https://codereview.chromium.org/1098123002/diff/100001/tools/release/search_related_commits.py#newcode69
tools/release/search_related_commits.py:69: search_range,
nit: indentation, align list items with content in [
https://codereview.chromium.org/1098123002/diff/100001/tools/release/search_related_commits.py#newcode143
tools/release/search_related_commits.py:143: def
_pretty_print_entry(hash, pre_text, verbose):
Format and readability - how about:
output = _git_execute(
options.git_dir,
["show",
"--quiet",
"--date=iso",
"--format=%ad # %H # %s",
hash],
verbose,
)
print pre_text + output.strip()
https://codereview.chromium.org/1098123002/
--
--
v8-dev mailing list
[email protected]
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 [email protected].
For more options, visit https://groups.google.com/d/optout.
