Noissue - updated to libadblockplus revision 139ae7a5e3ca

Noissue - updated to libadblockplus revision 139ae7a5e3ca

[sergei, 2017-04-07 07:57:31]

LGTM.

BTW, it also contains a couple of important fixes in implementation of timers,
so that's very good that we are already going to update libadblockplus.

[diegocarloslima, 2017-04-07 08:45:24]

Looks good, just some nits regarding doc.

https://codereview.adblockplus.org/29405555/diff/29405556/include/AdblockPlus...
File include/AdblockPlus/FilterEngine.h (right):

https://codereview.adblockplus.org/29405555/diff/29405556/include/AdblockPlus...
include/AdblockPlus/FilterEngine.h:124: * @return `true` if this subscription is
acceptable ads subscription.
maybe these should be replaced to 'is an acceptable ads subscription.' ? Also,
shouldn't Acceptable Ads be capitalized?

https://codereview.adblockplus.org/29405555/diff/29405556/include/AdblockPlus...
include/AdblockPlus/FilterEngine.h:328: * Checks whether Acceptable Ads
subscription is enabled.
I think it should be either 'whether an Acceptable Ads' or 'whether the
Acceptable Ads'

https://codereview.adblockplus.org/29405555/diff/29405556/include/AdblockPlus...
include/AdblockPlus/FilterEngine.h:335: * available even if subscription is not
add yet.
it should be 'is not added yet.'

[diegocarloslima, 2017-04-07 08:48:30]

https://codereview.adblockplus.org/29405555/diff/29405556/include/AdblockPlus...
File include/AdblockPlus/FilterEngine.h (right):

https://codereview.adblockplus.org/29405555/diff/29405556/include/AdblockPlus...
include/AdblockPlus/FilterEngine.h:124: * @return `true` if this subscription is
acceptable ads subscription.
On 2017/04/07 08:45:24, diegocarloslima wrote:
> maybe these should be replaced to 'is an acceptable ads subscription.' ? Also,
> shouldn't Acceptable Ads be capitalized?

Actually, it should be 'is the Acceptable Ads subscription.', since we have only
one AA subscription

https://codereview.adblockplus.org/29405555/diff/29405556/include/AdblockPlus...
include/AdblockPlus/FilterEngine.h:328: * Checks whether Acceptable Ads
subscription is enabled.
On 2017/04/07 08:45:24, diegocarloslima wrote:
> I think it should be either 'whether an Acceptable Ads' or 'whether the
> Acceptable Ads'

Actually, it should be 'whether the Acceptable Ads', since he have only one AA
subscription

[anton, 2017-04-07 09:34:19]

On 2017/04/07 08:48:30, diegocarloslima wrote:
>
https://codereview.adblockplus.org/29405555/diff/29405556/include/AdblockPlus...
> File include/AdblockPlus/FilterEngine.h (right):
> 
>
https://codereview.adblockplus.org/29405555/diff/29405556/include/AdblockPlus...
> include/AdblockPlus/FilterEngine.h:124: * @return `true` if this subscription
is
> acceptable ads subscription.
> On 2017/04/07 08:45:24, diegocarloslima wrote:
> > maybe these should be replaced to 'is an acceptable ads subscription.' ?
Also,
> > shouldn't Acceptable Ads be capitalized?
> 
> Actually, it should be 'is the Acceptable Ads subscription.', since we have
only
> one AA subscription
> 
>
https://codereview.adblockplus.org/29405555/diff/29405556/include/AdblockPlus...
> include/AdblockPlus/FilterEngine.h:328: * Checks whether Acceptable Ads
> subscription is enabled.
> On 2017/04/07 08:45:24, diegocarloslima wrote:
> > I think it should be either 'whether an Acceptable Ads' or 'whether the
> > Acceptable Ads'
> 
> Actually, it should be 'whether the Acceptable Ads', since he have only one AA
> subscription

It's merged from libadblcokplus revision changes between last revision and
https://hg.adblockplus.org/libadblockplus/rev/139ae7a5e3ca  so i don't expect it
to be changed

[diegocarloslima, 2017-04-07 09:53:36]

On 2017/04/07 09:34:19, anton wrote:
> On 2017/04/07 08:48:30, diegocarloslima wrote:
> >
>
https://codereview.adblockplus.org/29405555/diff/29405556/include/AdblockPlus...
> > File include/AdblockPlus/FilterEngine.h (right):
> > 
> >
>
https://codereview.adblockplus.org/29405555/diff/29405556/include/AdblockPlus...
> > include/AdblockPlus/FilterEngine.h:124: * @return `true` if this
subscription
> is
> > acceptable ads subscription.
> > On 2017/04/07 08:45:24, diegocarloslima wrote:
> > > maybe these should be replaced to 'is an acceptable ads subscription.' ?
> Also,
> > > shouldn't Acceptable Ads be capitalized?
> > 
> > Actually, it should be 'is the Acceptable Ads subscription.', since we have
> only
> > one AA subscription
> > 
> >
>
https://codereview.adblockplus.org/29405555/diff/29405556/include/AdblockPlus...
> > include/AdblockPlus/FilterEngine.h:328: * Checks whether Acceptable Ads
> > subscription is enabled.
> > On 2017/04/07 08:45:24, diegocarloslima wrote:
> > > I think it should be either 'whether an Acceptable Ads' or 'whether the
> > > Acceptable Ads'
> > 
> > Actually, it should be 'whether the Acceptable Ads', since he have only one
AA
> > subscription
> 
> It's merged from libadblcokplus revision changes between last revision and
> https://hg.adblockplus.org/libadblockplus/rev/139ae7a5e3ca  so i don't expect
it
> to be changed

I see, so it's LGTM then, but maybe a ticket should be created to fix those
small nits in libadblockplus and here later

[sergei, 2017-04-07 10:16:04]

A codereview with the fixes https://codereview.adblockplus.org/29405620/ in
comments.