https://codereview.chromium.org/1098123002/diff/120001/tools/release/search_related_commits.py
File tools/release/search_related_commits.py (right):
https://codereview.chromium.org/1098123002/diff/120001/tools/release/search_related_commits.py#newcode22
tools/release/search_related_commits.py:22: #Adding start hash too
On 2015/04/28 21:23:51, Michael Achenbach wrote:
nit: Space after # - end comments with periods.
Done.
https://codereview.chromium.org/1098123002/diff/120001/tools/release/search_related_commits.py#newcode78
tools/release/search_related_commits.py:78: git_working_dir,(
On 2015/04/28 21:23:51, Michael Achenbach wrote:
nit: space after comma
Done.
https://codereview.chromium.org/1098123002/diff/120001/tools/release/test_search_related_commits.py
File tools/release/test_search_related_commits.py (right):
https://codereview.chromium.org/1098123002/diff/120001/tools/release/test_search_related_commits.py#newcode6
tools/release/test_search_related_commits.py:6: import unittest
On 2015/04/28 21:23:51, Michael Achenbach wrote:
nit sort by module name
Done.
https://codereview.chromium.org/1098123002/diff/120001/tools/release/test_search_related_commits.py#newcode22
tools/release/test_search_related_commits.py:22: def _execute_git(self,
commands):
On 2015/04/28 21:23:51, Michael Achenbach wrote:
Maybe use *args for commands (e.g. *commands), then you don't need
lists below?
Acknowledged.
https://codereview.chromium.org/1098123002/diff/120001/tools/release/test_search_related_commits.py#newcode35
tools/release/test_search_related_commits.py:35: f = open(self.base_dir
+ "/" + file_name, 'w')
On 2015/04/28 21:23:51, Michael Achenbach wrote:
Use "with" statement
Acknowledged.
https://codereview.chromium.org/1098123002/diff/120001/tools/release/test_search_related_commits.py#newcode39
tools/release/test_search_related_commits.py:39:
self._execute_git(["add", self.base_dir + "/" + file_name])
On 2015/04/28 21:23:51, Michael Achenbach wrote:
Do you need files? There is a flag that allows empty commits
(--allow-empty?).
Do files give any benefit for these tests?
Acknowledged.
https://codereview.chromium.org/1098123002/diff/120001/tools/release/test_search_related_commits.py#newcode94
tools/release/test_search_related_commits.py:94: f = open(self.base_dir
+ "/first", 'w')
On 2015/04/28 21:23:51, Michael Achenbach wrote:
use "with" statement and/or extract some helper methods for similar
stuff. This
seems recurring.
Acknowledged.
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.
