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

Issue 29331148: Issue 3176 - Add metadata to content blocker lists (Closed)

Created:
Nov. 27, 2015, 4:22 p.m. by kzar
Modified:
Dec. 8, 2015, 4:01 p.m.
Visibility:
Public.

Description

Issue 3176 - Add metadata to content blocker lists

Patch Set 1 #

Total comments: 15

Patch Set 2 : Addressed feedback #

Patch Set 3 : Remove unused StringIO import #

Total comments: 4

Patch Set 4 : Addressed further feedback #

Total comments: 23

Patch Set 5 : Addressed more feedback from Felix and Sebastian #

Total comments: 8

Patch Set 6 : Addressed some of Sebastian's feedback #

Patch Set 7 : Just grab version number instead of parsing header #

Total comments: 3

Patch Set 8 : Improved regexp #

Unified diffs Side-by-side diffs Delta from patch set Stats (+54 lines, -35 lines) Patch
M .sitescripts.example View 1 1 chunk +2 lines, -0 lines 0 comments Download
M sitescripts/content_blocker_lists/bin/generate_lists.py View 1 2 3 4 5 6 7 1 chunk +52 lines, -35 lines 0 comments Download

Messages

Total messages: 25
kzar
Patch Set 1 https://codereview.adblockplus.org/29331148/diff/29331149/sitescripts/content_blocker_lists/bin/generate_lists.py File sitescripts/content_blocker_lists/bin/generate_lists.py (right): https://codereview.adblockplus.org/29331148/diff/29331149/sitescripts/content_blocker_lists/bin/generate_lists.py#newcode65 sitescripts/content_blocker_lists/bin/generate_lists.py:65: def generate_metadata(filter_lists, expires="4 days"): It is ...
Nov. 27, 2015, 4:28 p.m. (2015-11-27 16:28:11 UTC) #1
Sebastian Noack
https://codereview.adblockplus.org/29331148/diff/29331149/sitescripts/content_blocker_lists/bin/generate_lists.py File sitescripts/content_blocker_lists/bin/generate_lists.py (right): https://codereview.adblockplus.org/29331148/diff/29331149/sitescripts/content_blocker_lists/bin/generate_lists.py#newcode54 sitescripts/content_blocker_lists/bin/generate_lists.py:54: with closing(StringIO(filter_list)) as stream: Never mind closing a StringIO. ...
Nov. 30, 2015, 1:55 p.m. (2015-11-30 13:55:49 UTC) #2
kzar
Patch Set 2 : Addressed feedback https://codereview.adblockplus.org/29331148/diff/29331149/sitescripts/content_blocker_lists/bin/generate_lists.py File sitescripts/content_blocker_lists/bin/generate_lists.py (right): https://codereview.adblockplus.org/29331148/diff/29331149/sitescripts/content_blocker_lists/bin/generate_lists.py#newcode54 sitescripts/content_blocker_lists/bin/generate_lists.py:54: with closing(StringIO(filter_list)) as ...
Nov. 30, 2015, 3:13 p.m. (2015-11-30 15:13:12 UTC) #3
kzar
Patch Set 3 : Remove unused StringIO import
Nov. 30, 2015, 3:14 p.m. (2015-11-30 15:14:43 UTC) #4
Sebastian Noack
https://codereview.adblockplus.org/29331148/diff/29331149/sitescripts/content_blocker_lists/bin/generate_lists.py File sitescripts/content_blocker_lists/bin/generate_lists.py (right): https://codereview.adblockplus.org/29331148/diff/29331149/sitescripts/content_blocker_lists/bin/generate_lists.py#newcode54 sitescripts/content_blocker_lists/bin/generate_lists.py:54: with closing(StringIO(filter_list)) as stream: On 2015/11/30 15:13:11, kzar wrote: ...
Nov. 30, 2015, 3:49 p.m. (2015-11-30 15:49:19 UTC) #5
Sebastian Noack
https://codereview.adblockplus.org/29331148/diff/29331611/sitescripts/content_blocker_lists/bin/generate_lists.py File sitescripts/content_blocker_lists/bin/generate_lists.py (right): https://codereview.adblockplus.org/29331148/diff/29331611/sitescripts/content_blocker_lists/bin/generate_lists.py#newcode58 sitescripts/content_blocker_lists/bin/generate_lists.py:58: metadata = { Nit: Use an OrderedDict to make ...
Nov. 30, 2015, 4 p.m. (2015-11-30 16:00:07 UTC) #6
kzar
Patch Set 4 : Addressed further feedback https://codereview.adblockplus.org/29331148/diff/29331149/sitescripts/content_blocker_lists/bin/generate_lists.py File sitescripts/content_blocker_lists/bin/generate_lists.py (right): https://codereview.adblockplus.org/29331148/diff/29331149/sitescripts/content_blocker_lists/bin/generate_lists.py#newcode54 sitescripts/content_blocker_lists/bin/generate_lists.py:54: with closing(StringIO(filter_list)) ...
Nov. 30, 2015, 5:06 p.m. (2015-11-30 17:06:00 UTC) #7
Felix Dahlke
https://codereview.adblockplus.org/29331148/diff/29331149/sitescripts/content_blocker_lists/bin/generate_lists.py File sitescripts/content_blocker_lists/bin/generate_lists.py (right): https://codereview.adblockplus.org/29331148/diff/29331149/sitescripts/content_blocker_lists/bin/generate_lists.py#newcode54 sitescripts/content_blocker_lists/bin/generate_lists.py:54: with closing(StringIO(filter_list)) as stream: On 2015/11/30 17:06:00, kzar wrote: ...
Dec. 1, 2015, 8:32 a.m. (2015-12-01 08:32:56 UTC) #8
Felix Dahlke
Might as well have a look at the whole thing. Looks good, just some minor ...
Dec. 1, 2015, 8:43 a.m. (2015-12-01 08:43:09 UTC) #9
Sebastian Noack
https://codereview.adblockplus.org/29331148/diff/29331636/sitescripts/content_blocker_lists/bin/generate_lists.py File sitescripts/content_blocker_lists/bin/generate_lists.py (right): https://codereview.adblockplus.org/29331148/diff/29331636/sitescripts/content_blocker_lists/bin/generate_lists.py#newcode33 sitescripts/content_blocker_lists/bin/generate_lists.py:33: def update_abp2blocklist(): On 2015/12/01 08:43:09, Felix Dahlke wrote: > ...
Dec. 1, 2015, 10:18 a.m. (2015-12-01 10:18:40 UTC) #10
Felix Dahlke
https://codereview.adblockplus.org/29331148/diff/29331636/sitescripts/content_blocker_lists/bin/generate_lists.py File sitescripts/content_blocker_lists/bin/generate_lists.py (right): https://codereview.adblockplus.org/29331148/diff/29331636/sitescripts/content_blocker_lists/bin/generate_lists.py#newcode67 sitescripts/content_blocker_lists/bin/generate_lists.py:67: for k in ["url", "Version"]}) On 2015/12/01 10:18:39, Sebastian ...
Dec. 1, 2015, 10:26 a.m. (2015-12-01 10:26:29 UTC) #11
Sebastian Noack
https://codereview.adblockplus.org/29331148/diff/29331636/sitescripts/content_blocker_lists/bin/generate_lists.py File sitescripts/content_blocker_lists/bin/generate_lists.py (right): https://codereview.adblockplus.org/29331148/diff/29331636/sitescripts/content_blocker_lists/bin/generate_lists.py#newcode67 sitescripts/content_blocker_lists/bin/generate_lists.py:67: for k in ["url", "Version"]}) On 2015/12/01 10:26:29, Felix ...
Dec. 1, 2015, 10:38 a.m. (2015-12-01 10:38:10 UTC) #12
Sebastian Noack
https://codereview.adblockplus.org/29331148/diff/29331636/sitescripts/content_blocker_lists/bin/generate_lists.py File sitescripts/content_blocker_lists/bin/generate_lists.py (right): https://codereview.adblockplus.org/29331148/diff/29331636/sitescripts/content_blocker_lists/bin/generate_lists.py#newcode75 sitescripts/content_blocker_lists/bin/generate_lists.py:75: def pipe_in(process): Nit: This is inconsistent. You pass in ...
Dec. 1, 2015, 10:47 a.m. (2015-12-01 10:47:14 UTC) #13
Sebastian Noack
https://codereview.adblockplus.org/29331148/diff/29331636/sitescripts/content_blocker_lists/bin/generate_lists.py File sitescripts/content_blocker_lists/bin/generate_lists.py (right): https://codereview.adblockplus.org/29331148/diff/29331636/sitescripts/content_blocker_lists/bin/generate_lists.py#newcode51 sitescripts/content_blocker_lists/bin/generate_lists.py:51: return filter_list I think it would be simpler if ...
Dec. 1, 2015, 11:01 a.m. (2015-12-01 11:01:49 UTC) #14
kzar
Patch Set 5 : Addressed more feedback from Felix and Sebastian https://codereview.adblockplus.org/29331148/diff/29331149/sitescripts/content_blocker_lists/bin/generate_lists.py File sitescripts/content_blocker_lists/bin/generate_lists.py (right): ...
Dec. 1, 2015, 12:13 p.m. (2015-12-01 12:13:39 UTC) #15
Felix Dahlke
https://codereview.adblockplus.org/29331148/diff/29331149/sitescripts/content_blocker_lists/bin/generate_lists.py File sitescripts/content_blocker_lists/bin/generate_lists.py (right): https://codereview.adblockplus.org/29331148/diff/29331149/sitescripts/content_blocker_lists/bin/generate_lists.py#newcode54 sitescripts/content_blocker_lists/bin/generate_lists.py:54: with closing(StringIO(filter_list)) as stream: On 2015/12/01 12:13:38, kzar wrote: ...
Dec. 1, 2015, 2:04 p.m. (2015-12-01 14:04:20 UTC) #16
Sebastian Noack
https://codereview.adblockplus.org/29331148/diff/29331636/sitescripts/content_blocker_lists/bin/generate_lists.py File sitescripts/content_blocker_lists/bin/generate_lists.py (right): https://codereview.adblockplus.org/29331148/diff/29331636/sitescripts/content_blocker_lists/bin/generate_lists.py#newcode33 sitescripts/content_blocker_lists/bin/generate_lists.py:33: def update_abp2blocklist(): On 2015/12/01 14:04:19, Felix Dahlke wrote: > ...
Dec. 7, 2015, 12:38 p.m. (2015-12-07 12:38:25 UTC) #17
Felix Dahlke
https://codereview.adblockplus.org/29331148/diff/29331636/sitescripts/content_blocker_lists/bin/generate_lists.py File sitescripts/content_blocker_lists/bin/generate_lists.py (right): https://codereview.adblockplus.org/29331148/diff/29331636/sitescripts/content_blocker_lists/bin/generate_lists.py#newcode33 sitescripts/content_blocker_lists/bin/generate_lists.py:33: def update_abp2blocklist(): On 2015/12/07 12:38:24, Sebastian Noack wrote: > ...
Dec. 8, 2015, 6:51 a.m. (2015-12-08 06:51:24 UTC) #18
kzar
Patch Set 6 : Addressed some of Sebastian's feedback https://codereview.adblockplus.org/29331148/diff/29331684/sitescripts/content_blocker_lists/bin/generate_lists.py File sitescripts/content_blocker_lists/bin/generate_lists.py (right): https://codereview.adblockplus.org/29331148/diff/29331684/sitescripts/content_blocker_lists/bin/generate_lists.py#newcode47 sitescripts/content_blocker_lists/bin/generate_lists.py:47: ...
Dec. 8, 2015, 12:52 p.m. (2015-12-08 12:52:21 UTC) #19
Felix Dahlke
https://codereview.adblockplus.org/29331148/diff/29331684/sitescripts/content_blocker_lists/bin/generate_lists.py File sitescripts/content_blocker_lists/bin/generate_lists.py (right): https://codereview.adblockplus.org/29331148/diff/29331684/sitescripts/content_blocker_lists/bin/generate_lists.py#newcode47 sitescripts/content_blocker_lists/bin/generate_lists.py:47: field_re = re.compile(r"^!\s*([^:\s]+):\s*(.+)$", re.MULTILINE) On 2015/12/08 12:52:21, kzar wrote: ...
Dec. 8, 2015, 1:39 p.m. (2015-12-08 13:39:51 UTC) #20
kzar
Patch Set 7 : Just grab version number instead of parsing header https://codereview.adblockplus.org/29331148/diff/29331684/sitescripts/content_blocker_lists/bin/generate_lists.py File sitescripts/content_blocker_lists/bin/generate_lists.py ...
Dec. 8, 2015, 2:34 p.m. (2015-12-08 14:34:24 UTC) #21
Sebastian Noack
https://codereview.adblockplus.org/29331148/diff/29332446/sitescripts/content_blocker_lists/bin/generate_lists.py File sitescripts/content_blocker_lists/bin/generate_lists.py (right): https://codereview.adblockplus.org/29331148/diff/29332446/sitescripts/content_blocker_lists/bin/generate_lists.py#newcode48 sitescripts/content_blocker_lists/bin/generate_lists.py:48: version = re.search(r"^(?:[^[!])|!\s*Version:\s*(.+)$", Indeed, we can simply use re.search ...
Dec. 8, 2015, 3:08 p.m. (2015-12-08 15:08:39 UTC) #22
kzar
Patch Set 8 : Improved regexp https://codereview.adblockplus.org/29331148/diff/29332446/sitescripts/content_blocker_lists/bin/generate_lists.py File sitescripts/content_blocker_lists/bin/generate_lists.py (right): https://codereview.adblockplus.org/29331148/diff/29332446/sitescripts/content_blocker_lists/bin/generate_lists.py#newcode48 sitescripts/content_blocker_lists/bin/generate_lists.py:48: version = re.search(r"^(?:[^[!])|!\s*Version:\s*(.+)$", ...
Dec. 8, 2015, 3:37 p.m. (2015-12-08 15:37:44 UTC) #23
Sebastian Noack
https://codereview.adblockplus.org/29331148/diff/29332446/sitescripts/content_blocker_lists/bin/generate_lists.py File sitescripts/content_blocker_lists/bin/generate_lists.py (right): https://codereview.adblockplus.org/29331148/diff/29332446/sitescripts/content_blocker_lists/bin/generate_lists.py#newcode48 sitescripts/content_blocker_lists/bin/generate_lists.py:48: version = re.search(r"^(?:[^[!])|!\s*Version:\s*(.+)$", On 2015/12/08 15:37:43, kzar wrote: > ...
Dec. 8, 2015, 3:45 p.m. (2015-12-08 15:45:52 UTC) #24
Felix Dahlke
Dec. 8, 2015, 3:47 p.m. (2015-12-08 15:47:29 UTC) #25
LGTM

Powered by Google App Engine
This is Rietveld