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

Issue 29674555: Issue 6273 - Apply eyeo python code style (Closed)

Created:
Jan. 19, 2018, 7:28 a.m. by anton
Modified:
Jan. 30, 2018, 7:06 a.m.
CC:
René Jeschke, sergei
Visibility:
Public.

Description

Issue 6273 - Apply eyeo python code style

Patch Set 1 #

Total comments: 26

Patch Set 2 : Addressed Vasily's comments #

Total comments: 14

Patch Set 3 : Removed sys.exit() where return code is always 0 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+98 lines, -123 lines) Patch
M third_party/libadblockplus/download_ndk.py View 1 2 2 chunks +5 lines, -6 lines 0 comments Download
M third_party/libadblockplus/prepare_dependencies.py View 1 2 1 chunk +80 lines, -103 lines 0 comments Download
M third_party/libadblockplus_android/prepare_build_tools.py View 1 2 chunks +4 lines, -3 lines 0 comments Download
M third_party/libadblockplus_common/delete_dir.py View 1 2 1 chunk +3 lines, -6 lines 0 comments Download
M third_party/libadblockplus_common/subproc.py View 1 4 chunks +6 lines, -5 lines 0 comments Download

Messages

Total messages: 19
anton
https://codereview.adblockplus.org/29674555/diff/29674556/third_party/libadblockplus/prepare_dependencies.py File third_party/libadblockplus/prepare_dependencies.py (right): https://codereview.adblockplus.org/29674555/diff/29674556/third_party/libadblockplus/prepare_dependencies.py#newcode30 third_party/libadblockplus/prepare_dependencies.py:30: libadblockplus_root = os.path.join(cwd, It would be better to rename ...
Jan. 19, 2018, 7:30 a.m. (2018-01-19 07:30:20 UTC) #1
Vasily Kuznetsov
Hi Anton, The script looks pretty good. I have a bunch of comments, but most ...
Jan. 19, 2018, 1:06 p.m. (2018-01-19 13:06:53 UTC) #2
anton
https://codereview.adblockplus.org/29674555/diff/29674556/third_party/libadblockplus/prepare_dependencies.py File third_party/libadblockplus/prepare_dependencies.py (right): https://codereview.adblockplus.org/29674555/diff/29674556/third_party/libadblockplus/prepare_dependencies.py#newcode6 third_party/libadblockplus/prepare_dependencies.py:6: def duplicate(src, dst, symlink=False): On 2018/01/19 13:06:53, Vasily Kuznetsov ...
Jan. 19, 2018, 1:17 p.m. (2018-01-19 13:17:15 UTC) #3
Vasily Kuznetsov
https://codereview.adblockplus.org/29674555/diff/29674556/third_party/libadblockplus/prepare_dependencies.py File third_party/libadblockplus/prepare_dependencies.py (right): https://codereview.adblockplus.org/29674555/diff/29674556/third_party/libadblockplus/prepare_dependencies.py#newcode9 third_party/libadblockplus/prepare_dependencies.py:9: print('Deleting {}'.format(dst)) On 2018/01/19 13:17:15, anton wrote: > On ...
Jan. 19, 2018, 1:35 p.m. (2018-01-19 13:35:17 UTC) #4
anton
On 2018/01/19 13:35:17, Vasily Kuznetsov wrote: > https://codereview.adblockplus.org/29674555/diff/29674556/third_party/libadblockplus/prepare_dependencies.py > File third_party/libadblockplus/prepare_dependencies.py (right): > > https://codereview.adblockplus.org/29674555/diff/29674556/third_party/libadblockplus/prepare_dependencies.py#newcode9 ...
Jan. 19, 2018, 1:37 p.m. (2018-01-19 13:37:49 UTC) #5
sergei
https://codereview.adblockplus.org/29674555/diff/29674556/third_party/libadblockplus/prepare_dependencies.py File third_party/libadblockplus/prepare_dependencies.py (right): https://codereview.adblockplus.org/29674555/diff/29674556/third_party/libadblockplus/prepare_dependencies.py#newcode30 third_party/libadblockplus/prepare_dependencies.py:30: libadblockplus_root = os.path.join(cwd, On 2018/01/19 13:35:17, Vasily Kuznetsov wrote: ...
Jan. 19, 2018, 2:19 p.m. (2018-01-19 14:19:43 UTC) #6
anton
On 2018/01/19 14:19:43, sergei wrote: > https://codereview.adblockplus.org/29674555/diff/29674556/third_party/libadblockplus/prepare_dependencies.py > File third_party/libadblockplus/prepare_dependencies.py (right): > > https://codereview.adblockplus.org/29674555/diff/29674556/third_party/libadblockplus/prepare_dependencies.py#newcode30 > ...
Jan. 22, 2018, 7:12 a.m. (2018-01-22 07:12:41 UTC) #7
sergei
https://codereview.adblockplus.org/29674555/diff/29676583/third_party/libadblockplus/prepare_dependencies.py File third_party/libadblockplus/prepare_dependencies.py (right): https://codereview.adblockplus.org/29674555/diff/29676583/third_party/libadblockplus/prepare_dependencies.py#newcode8 third_party/libadblockplus/prepare_dependencies.py:8: def duplicate(src, dst, symlink=False): It seems that symlink parameter ...
Jan. 22, 2018, 10:08 a.m. (2018-01-22 10:08:59 UTC) #8
sergei
https://codereview.adblockplus.org/29674555/diff/29676583/third_party/libadblockplus/prepare_dependencies.py File third_party/libadblockplus/prepare_dependencies.py (right): https://codereview.adblockplus.org/29674555/diff/29676583/third_party/libadblockplus/prepare_dependencies.py#newcode8 third_party/libadblockplus/prepare_dependencies.py:8: def duplicate(src, dst, symlink=False): BTW, what about moving this ...
Jan. 22, 2018, 10:10 a.m. (2018-01-22 10:10:33 UTC) #9
anton
https://codereview.adblockplus.org/29674555/diff/29676583/third_party/libadblockplus/prepare_dependencies.py File third_party/libadblockplus/prepare_dependencies.py (right): https://codereview.adblockplus.org/29674555/diff/29676583/third_party/libadblockplus/prepare_dependencies.py#newcode8 third_party/libadblockplus/prepare_dependencies.py:8: def duplicate(src, dst, symlink=False): On 2018/01/22 10:08:59, sergei wrote: ...
Jan. 22, 2018, 10:18 a.m. (2018-01-22 10:18:42 UTC) #10
sergei
https://codereview.adblockplus.org/29674555/diff/29676583/third_party/libadblockplus/prepare_dependencies.py File third_party/libadblockplus/prepare_dependencies.py (right): https://codereview.adblockplus.org/29674555/diff/29676583/third_party/libadblockplus/prepare_dependencies.py#newcode8 third_party/libadblockplus/prepare_dependencies.py:8: def duplicate(src, dst, symlink=False): On 2018/01/22 10:18:41, anton wrote: ...
Jan. 22, 2018, 12:04 p.m. (2018-01-22 12:04:03 UTC) #11
anton
On 2018/01/22 12:04:03, sergei wrote: > https://codereview.adblockplus.org/29674555/diff/29676583/third_party/libadblockplus/prepare_dependencies.py > File third_party/libadblockplus/prepare_dependencies.py (right): > > https://codereview.adblockplus.org/29674555/diff/29676583/third_party/libadblockplus/prepare_dependencies.py#newcode8 > ...
Jan. 22, 2018, 12:55 p.m. (2018-01-22 12:55:20 UTC) #12
sergei
https://codereview.adblockplus.org/29674555/diff/29676583/third_party/libadblockplus/prepare_dependencies.py File third_party/libadblockplus/prepare_dependencies.py (right): https://codereview.adblockplus.org/29674555/diff/29676583/third_party/libadblockplus/prepare_dependencies.py#newcode70 third_party/libadblockplus/prepare_dependencies.py:70: for path in [ On 2018/01/22 12:04:02, sergei wrote: ...
Jan. 24, 2018, 5:23 p.m. (2018-01-24 17:23:41 UTC) #13
anton
On 2018/01/24 17:23:41, sergei wrote: > https://codereview.adblockplus.org/29674555/diff/29676583/third_party/libadblockplus/prepare_dependencies.py > File third_party/libadblockplus/prepare_dependencies.py (right): > > https://codereview.adblockplus.org/29674555/diff/29676583/third_party/libadblockplus/prepare_dependencies.py#newcode70 > ...
Jan. 25, 2018, 10:56 a.m. (2018-01-25 10:56:29 UTC) #14
anton
On 2018/01/25 10:56:29, anton wrote: > On 2018/01/24 17:23:41, sergei wrote: > > > https://codereview.adblockplus.org/29674555/diff/29676583/third_party/libadblockplus/prepare_dependencies.py ...
Jan. 29, 2018, 6:03 a.m. (2018-01-29 06:03:27 UTC) #15
Vasily Kuznetsov
I have one minor comment remaining, but basically it looks good. LGTM. https://codereview.adblockplus.org/29674555/diff/29676583/third_party/libadblockplus/prepare_dependencies.py File third_party/libadblockplus/prepare_dependencies.py ...
Jan. 29, 2018, 12:39 p.m. (2018-01-29 12:39:13 UTC) #16
anton
https://codereview.adblockplus.org/29674555/diff/29676583/third_party/libadblockplus/prepare_dependencies.py File third_party/libadblockplus/prepare_dependencies.py (right): https://codereview.adblockplus.org/29674555/diff/29676583/third_party/libadblockplus/prepare_dependencies.py#newcode88 third_party/libadblockplus/prepare_dependencies.py:88: sys.exit(prepare_deps()) On 2018/01/29 12:39:12, Vasily Kuznetsov wrote: > As ...
Jan. 29, 2018, 12:52 p.m. (2018-01-29 12:52:54 UTC) #17
diegocarloslima
On 2018/01/29 12:52:54, anton wrote: > https://codereview.adblockplus.org/29674555/diff/29676583/third_party/libadblockplus/prepare_dependencies.py > File third_party/libadblockplus/prepare_dependencies.py (right): > > https://codereview.adblockplus.org/29674555/diff/29676583/third_party/libadblockplus/prepare_dependencies.py#newcode88 > ...
Jan. 29, 2018, 3:13 p.m. (2018-01-29 15:13:23 UTC) #18
Vasily Kuznetsov
Jan. 29, 2018, 4:27 p.m. (2018-01-29 16:27:57 UTC) #19
> See patch set #3 (thought i've left few files unchanged where return code is
not
> always 0)

Even more LGTM!

Powered by Google App Engine
This is Rietveld