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

Issue 29438577: Issue 5248 - Use wildcard to match any subdomains (Closed)

Created:
May 16, 2017, 1:06 a.m. by Manish Jethani
Modified:
May 16, 2017, 1:25 p.m.
Reviewers:
Sebastian Noack, kzar
Base URL:
https://hg.adblockplus.org/abp2blocklist
Visibility:
Public.

Description

Remove the addDomainPrefix function and the tldjs dependency that came with it. Instead, simply add a * character in front of the name. e.g. $domain=example.com should get converted to "if-domain": ["*example.com"]

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+10 lines, -27 lines) Patch
M lib/abp2blocklist.js View 3 chunks +3 lines, -19 lines 0 comments Download
M package.json View 1 chunk +1 line, -2 lines 0 comments Download
M test/abp2blocklist.js View 2 chunks +6 lines, -6 lines 1 comment Download

Messages

Total messages: 5
Manish Jethani
May 16, 2017, 1:06 a.m. (2017-05-16 01:06:11 UTC) #1
Manish Jethani
Patch Set 1 https://codereview.adblockplus.org/29438577/diff/29438578/test/abp2blocklist.js File test/abp2blocklist.js (left): https://codereview.adblockplus.org/29438577/diff/29438578/test/abp2blocklist.js#oldcode218 test/abp2blocklist.js:218: testRules(test, ["2$domain=third-party"], ["third-party"], This test was ...
May 16, 2017, 1:08 a.m. (2017-05-16 01:08:27 UTC) #2
Sebastian Noack
LGTM
May 16, 2017, 7:09 a.m. (2017-05-16 07:09:45 UTC) #3
kzar
LGTM but please file an issue for the change before pushing. It's an important enough ...
May 16, 2017, 8:54 a.m. (2017-05-16 08:54:06 UTC) #4
Manish Jethani
May 16, 2017, 11:29 a.m. (2017-05-16 11:29:05 UTC) #5
On 2017/05/16 08:54:06, kzar wrote:
> LGTM but please file an issue for the change before pushing. It's an important
> enough change to warrant an issue.

Makes sense, I've created #5248 for this now.

Powered by Google App Engine
This is Rietveld