Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 29350244: Issue 4374 - Made Python code part of jshydra comply with our coding practices (Closed)

Created:
Aug. 28, 2016, 7:40 p.m. by Sebastian Noack
Modified:
Aug. 30, 2016, 1:47 p.m.
Reviewers:
Vasily Kuznetsov
Visibility:
Public.

Description

Issue 4374 - Made Python code part of jshydra comply with our coding practices

Patch Set 1 : #

Patch Set 2 : Merge abp_rewrite and utils module #

Total comments: 1

Patch Set 3 : Rebased and fixed typo #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+98 lines, -96 lines) Patch
M abp_rewrite.py View 1 2 1 chunk +70 lines, -2 lines 0 comments Download
M autotest.py View 1 2 2 chunks +22 lines, -18 lines 2 comments Download
M tox.ini View 1 2 2 chunks +6 lines, -0 lines 0 comments Download
R utils.py View 1 2 1 chunk +0 lines, -76 lines 0 comments Download

Messages

Total messages: 4
Sebastian Noack
https://codereview.adblockplus.org/29350244/diff/29350272/abp_rewrite.py File abp_rewrite.py (right): https://codereview.adblockplus.org/29350244/diff/29350272/abp_rewrite.py#newcode8 abp_rewrite.py:8: import platform All I did with patchset #2 is ...
Aug. 28, 2016, 10:51 p.m. (2016-08-28 22:51:56 UTC) #1
Vasily Kuznetsov
LGTM. Thank you for splitting it into two patchsets this way -- indeed easier to ...
Aug. 29, 2016, 5:02 p.m. (2016-08-29 17:02:35 UTC) #2
Sebastian Noack
https://codereview.adblockplus.org/29350244/diff/29350311/autotest.py File autotest.py (right): https://codereview.adblockplus.org/29350244/diff/29350311/autotest.py#newcode34 autotest.py:34: elif key == 'arguments': I accidentally replaced this string ...
Aug. 30, 2016, 12:03 p.m. (2016-08-30 12:03:07 UTC) #3
Vasily Kuznetsov
Aug. 30, 2016, 1:25 p.m. (2016-08-30 13:25:30 UTC) #4
LGTM

https://codereview.adblockplus.org/29350244/diff/29350311/autotest.py
File autotest.py (right):

https://codereview.adblockplus.org/29350244/diff/29350311/autotest.py#newcode34
autotest.py:34: elif key == 'arguments':
On 2016/08/30 12:03:06, Sebastian Noack wrote:
> I accidentally replaced this string as well when renaming the variable,
causing
> no tests to be found. All other changes in this patch set are due to rebasing.

Oops, I missed it too. Good catch!

Powered by Google App Engine
This is Rietveld