LGTM with comments. Happy to take another look if you end up making significant
changes.

https://codereview.chromium.org/597943002/diff/1/tools/check-name-clashes.py
File tools/check-name-clashes.py (right):

https://codereview.chromium.org/597943002/diff/1/tools/check-name-clashes.py#newcode13
tools/check-name-clashes.py:13: LISTBODY = re.compile(".*\\\\$")
nit: if you use Python's r"..." notation, that'll save you one level of
backslash-escaping.
Also, I think you want to make the trailing backslash optional, or you
won't match the last line in each macro.

https://codereview.chromium.org/597943002/diff/1/tools/check-name-clashes.py#newcode43
tools/check-name-clashes.py:43: for line in lines:
I'd inline ReadFile here and iterate over the opened file's lines
directly.

https://codereview.chromium.org/597943002/diff/1/tools/check-name-clashes.py#newcode63
tools/check-name-clashes.py:63: if ListName(list) in BLACKLIST:
I'd be inclined to move this logic into the first line matching loop as
well, but I'm not sure it's worth it (since it makes the logic there
more complex, and doesn't really avoid any computation), so I'm leaving
it up to you.

https://codereview.chromium.org/597943002/diff/1/tools/check-name-clashes.py#newcode70
tools/check-name-clashes.py:70: print(match.group(1))
debugging leftover?

https://codereview.chromium.org/597943002/

--
--
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.

Reply via email to