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

Issue 29670562: Issue 6272 - Can't sync because of changes in android_tools (Closed)

Created:
Jan. 16, 2018, 7:43 a.m. by anton
Modified:
Jan. 19, 2018, 6:38 a.m.
CC:
Felix Dahlke
Visibility:
Public.

Description

Issue 6272 - Can't sync because of changes in android_tools Implemented in the simplest way: https://issues.adblockplus.org/ticket/6272#comment:1 To be updated to respect eyeo python code style.

Patch Set 1 #

Patch Set 2 : Formatted with respect to eyeo python code style. Tested with flake8 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+68 lines, -37 lines) Patch
M third_party/libadblockplus_android/BUILD.gn View 1 chunk +1 line, -1 line 0 comments Download
M third_party/libadblockplus_android/prepare_build_tools.py View 1 1 chunk +67 lines, -36 lines 2 comments Download

Messages

Total messages: 6
anton
Jan. 16, 2018, 7:46 a.m. (2018-01-16 07:46:08 UTC) #1
anton
On 2018/01/16 07:46:08, anton wrote: Uploaded patch set #2
Jan. 16, 2018, 9 a.m. (2018-01-16 09:00:53 UTC) #2
diegocarloslima
https://codereview.adblockplus.org/29670562/diff/29670572/third_party/libadblockplus_android/prepare_build_tools.py File third_party/libadblockplus_android/prepare_build_tools.py (right): https://codereview.adblockplus.org/29670562/diff/29670572/third_party/libadblockplus_android/prepare_build_tools.py#newcode85 third_party/libadblockplus_android/prepare_build_tools.py:85: sys.exit(1) Some of the changes here could go in ...
Jan. 17, 2018, 1:08 p.m. (2018-01-17 13:08:54 UTC) #3
anton
https://codereview.adblockplus.org/29670562/diff/29670572/third_party/libadblockplus_android/prepare_build_tools.py File third_party/libadblockplus_android/prepare_build_tools.py (right): https://codereview.adblockplus.org/29670562/diff/29670572/third_party/libadblockplus_android/prepare_build_tools.py#newcode85 third_party/libadblockplus_android/prepare_build_tools.py:85: sys.exit(1) On 2018/01/17 13:08:53, diegocarloslima wrote: > Some of ...
Jan. 17, 2018, 1:12 p.m. (2018-01-17 13:12:11 UTC) #4
diegocarloslima
On 2018/01/17 13:12:11, anton wrote: > https://codereview.adblockplus.org/29670562/diff/29670572/third_party/libadblockplus_android/prepare_build_tools.py > File third_party/libadblockplus_android/prepare_build_tools.py (right): > > https://codereview.adblockplus.org/29670562/diff/29670572/third_party/libadblockplus_android/prepare_build_tools.py#newcode85 > ...
Jan. 18, 2018, 1:37 p.m. (2018-01-18 13:37:57 UTC) #5
jens
Jan. 18, 2018, 1:57 p.m. (2018-01-18 13:57:55 UTC) #6
On 2018/01/18 13:37:57, diegocarloslima wrote:
> On 2018/01/17 13:12:11, anton wrote:
> >
>
https://codereview.adblockplus.org/29670562/diff/29670572/third_party/libadbl...
> > File third_party/libadblockplus_android/prepare_build_tools.py (right):
> > 
> >
>
https://codereview.adblockplus.org/29670562/diff/29670572/third_party/libadbl...
> > third_party/libadblockplus_android/prepare_build_tools.py:85: sys.exit(1)
> > On 2018/01/17 13:08:53, diegocarloslima wrote:
> > > Some of the changes here could go in a refactoring NoIssue review, since
> they
> > > adjust things that were already commited and not directly related to this
> > issue.
> > > I would only insist if we have other python files that should also be
> > refactored
> > > in order to be in compliance with our coding style. Then, it would make
> sense
> > to
> > > adjust them all in a single code review.
> > 
> > I've created separate task for applying python code style:
> > https://issues.adblockplus.org/ticket/6273
> 
> OK, LGTM then

LGTM

Powered by Google App Engine
This is Rietveld