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

Issue 29646595: Issue 6214 - Configure libadblockplus binaries directory (Closed)

Created:
Dec. 21, 2017, 3:21 p.m. by anton
Modified:
Jan. 10, 2018, 2:26 p.m.
CC:
sergei
Visibility:
Public.

Description

Issue 6214 - Configure libadblockplus binaries directory

Patch Set 1 #

Total comments: 1

Patch Set 2 : renamed vars #

Unified diffs Side-by-side diffs Delta from patch set Stats (+24 lines, -10 lines) Patch
M README.md View 1 1 chunk +4 lines, -1 line 0 comments Download
M libadblockplus-android/jni/Android.mk View 1 10 chunks +20 lines, -9 lines 0 comments Download

Messages

Total messages: 7
anton
Dec. 21, 2017, 3:22 p.m. (2017-12-21 15:22:43 UTC) #1
diegocarloslima
https://codereview.adblockplus.org/29646595/diff/29646596/libadblockplus-android/jni/Android.mk File libadblockplus-android/jni/Android.mk (right): https://codereview.adblockplus.org/29646595/diff/29646596/libadblockplus-android/jni/Android.mk#newcode3 libadblockplus-android/jni/Android.mk:3: # SHARED_V8_LIB_DIRECTORY is expected to be full absolute path ...
Jan. 3, 2018, 11:49 a.m. (2018-01-03 11:49:30 UTC) #2
diegocarloslima
On 2018/01/03 11:49:30, diegocarloslima wrote: > https://codereview.adblockplus.org/29646595/diff/29646596/libadblockplus-android/jni/Android.mk > File libadblockplus-android/jni/Android.mk (right): > > https://codereview.adblockplus.org/29646595/diff/29646596/libadblockplus-android/jni/Android.mk#newcode3 > ...
Jan. 3, 2018, 11:49 a.m. (2018-01-03 11:49:35 UTC) #3
anton
On 2018/01/03 11:49:30, diegocarloslima wrote: > https://codereview.adblockplus.org/29646595/diff/29646596/libadblockplus-android/jni/Android.mk > File libadblockplus-android/jni/Android.mk (right): > > https://codereview.adblockplus.org/29646595/diff/29646596/libadblockplus-android/jni/Android.mk#newcode3 > ...
Jan. 9, 2018, 6:22 a.m. (2018-01-09 06:22:55 UTC) #4
jens
On 2018/01/09 06:22:55, anton wrote: > On 2018/01/03 11:49:30, diegocarloslima wrote: > > > https://codereview.adblockplus.org/29646595/diff/29646596/libadblockplus-android/jni/Android.mk ...
Jan. 10, 2018, 1:40 p.m. (2018-01-10 13:40:18 UTC) #5
anton
On 2018/01/10 13:40:18, jens wrote: > On 2018/01/09 06:22:55, anton wrote: > > On 2018/01/03 ...
Jan. 10, 2018, 1:49 p.m. (2018-01-10 13:49:42 UTC) #6
jens
Jan. 10, 2018, 1:53 p.m. (2018-01-10 13:53:50 UTC) #7 
On 2018/01/10 13:49:42, anton wrote:
> On 2018/01/10 13:40:18, jens wrote:
> > On 2018/01/09 06:22:55, anton wrote:
> > > On 2018/01/03 11:49:30, diegocarloslima wrote:
> > > >
> > >
> >
>
https://codereview.adblockplus.org/29646595/diff/29646596/libadblockplus-andr...
> > > > File libadblockplus-android/jni/Android.mk (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.adblockplus.org/29646595/diff/29646596/libadblockplus-andr...
> > > > libadblockplus-android/jni/Android.mk:3: # SHARED_V8_LIB_DIRECTORY is
> > expected
> > > > to be full absolute path if set by user
> > > > I wouldn't insist, but maybe it might be better to rename this var to
> > > > `SHARED_V8_LIB_DIR` just to be a little bit more concise
> > > 
> > > In general, i agree and i don't insist on my var name.
> > > Let's choose one according to Jens/Rene's preference.
> > If I had to choose, I would vote for SHARED_V8_LIB_DIR as it's shorter and
as
> > evident as SHARED_V8_LIB_DIRECTORY
> 
> See new patch set

LGTM

Powered by Google App Engine
This is Rietveld