Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(830)

Issue 29441592: Issue 4329 - Add $genericblock support (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 months ago by Manish Jethani
Modified:
3 months ago
Reviewers:
Sebastian Noack, kzar
Base URL:
https://hg.adblockplus.org/abp2blocklist
Visibility:
Public.

Description

Issue 4329 - Add $genericblock support This affects 23,579 rules in EasyList+AA

Patch Set 1 #

Patch Set 2 : Add unit tests #

Total comments: 2

Patch Set 3 : Fix bug: Make a copy of the exceptionDomains array #

Total comments: 2

Patch Set 4 : Add $genericblock domains after filter-specific exception domains #

Total comments: 2

Patch Set 5 : Rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+44 lines, -3 lines) Patch
M lib/abp2blocklist.js View 1 2 3 4 5 chunks +23 lines, -2 lines 0 comments Download
M test/abp2blocklist.js View 1 2 3 4 2 chunks +21 lines, -1 line 0 comments Download

Messages

Total messages: 14
Manish Jethani
3 months ago (2017-05-19 01:45:22 UTC) #1
Manish Jethani
Patch Set 1
3 months ago (2017-05-19 01:47:26 UTC) #2
Manish Jethani
Patch Set 2: Add unit tests
3 months ago (2017-05-19 02:12:32 UTC) #3
kzar
LGTM https://codereview.adblockplus.org/29441592/diff/29441595/lib/abp2blocklist.js File lib/abp2blocklist.js (right): https://codereview.adblockplus.org/29441592/diff/29441595/lib/abp2blocklist.js#newcode394 lib/abp2blocklist.js:394: if (filter.contentType & typeMap.GENERICBLOCK) Nit: The indentation looks ...
3 months ago (2017-05-19 12:05:39 UTC) #4
Manish Jethani
https://codereview.adblockplus.org/29441592/diff/29441595/lib/abp2blocklist.js File lib/abp2blocklist.js (right): https://codereview.adblockplus.org/29441592/diff/29441595/lib/abp2blocklist.js#newcode394 lib/abp2blocklist.js:394: if (filter.contentType & typeMap.GENERICBLOCK) On 2017/05/19 12:05:38, kzar wrote: ...
3 months ago (2017-05-20 00:22:10 UTC) #5
Manish Jethani
Patch Set 3: Fix bug: Make a copy of the exceptionsDomains array We have to ...
3 months ago (2017-05-20 02:00:57 UTC) #6
Sebastian Noack
https://codereview.adblockplus.org/29441592/diff/29442771/lib/abp2blocklist.js File lib/abp2blocklist.js (right): https://codereview.adblockplus.org/29441592/diff/29442771/lib/abp2blocklist.js#newcode256 lib/abp2blocklist.js:256: let excluded = exceptionDomains && exceptionDomains.slice() || []; Nit: ...
3 months ago (2017-05-20 06:25:29 UTC) #7
Manish Jethani
Patch Set 4: Add $genericblock domains after filter-specific exception domains https://codereview.adblockplus.org/29441592/diff/29442771/lib/abp2blocklist.js File lib/abp2blocklist.js (right): https://codereview.adblockplus.org/29441592/diff/29442771/lib/abp2blocklist.js#newcode256 ...
3 months ago (2017-05-20 19:30:23 UTC) #8
Manish Jethani
On 2017/05/20 19:30:23, Manish Jethani wrote: > Actually now that I think of it, the ...
3 months ago (2017-05-20 19:32:53 UTC) #9
Sebastian Noack
https://codereview.adblockplus.org/29441592/diff/29443570/lib/abp2blocklist.js File lib/abp2blocklist.js (right): https://codereview.adblockplus.org/29441592/diff/29443570/lib/abp2blocklist.js#newcode261 lib/abp2blocklist.js:261: excluded = excluded.concat(exceptionDomains); I wonder whether it would be ...
3 months ago (2017-05-21 20:44:35 UTC) #10
Manish Jethani
https://codereview.adblockplus.org/29441592/diff/29443570/lib/abp2blocklist.js File lib/abp2blocklist.js (right): https://codereview.adblockplus.org/29441592/diff/29443570/lib/abp2blocklist.js#newcode261 lib/abp2blocklist.js:261: excluded = excluded.concat(exceptionDomains); On 2017/05/21 20:44:35, Sebastian Noack wrote: ...
3 months ago (2017-05-21 21:03:57 UTC) #11
Sebastian Noack
LGTM
3 months ago (2017-05-21 21:05:04 UTC) #12
Manish Jethani
Patch Set 5: Rebase
3 months ago (2017-05-21 21:27:51 UTC) #13
Sebastian Noack
3 months ago (2017-05-21 21:31:23 UTC) #14
Still LGTM
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87257f5