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

Issue 5636796054503424: Issue 1170 - [adblockplus.org Anwiki to CMS migration] Migrate content (Closed)

Created:
Sept. 17, 2014, 8:33 a.m. by kzar
Modified:
March 8, 2015, 4:29 a.m.
Visibility:
Public.

Description

Issue #1170 adblockplus.org migration from Anwiki to our CMS. To test you need to first run the website-conversion script which is available in bitbucket: https://github.com/kzar/adblockplus-website-converter (Read the README.md in that repository for usage instructions.)

Patch Set 1 #

Total comments: 26

Patch Set 2 : Improvements to parse_interface filter regarding Wladimir's feedback #

Patch Set 3 : Brought in changes to website made over last few months. #

Total comments: 37

Patch Set 4 : Addressed feedback. #

Total comments: 2

Patch Set 5 : Removed a couple of spaces before filters I missed earlier. #

Patch Set 6 : Removed unescape filter. #

Patch Set 7 : Removed redundant safe filters #

Total comments: 54

Patch Set 8 : Addressed palant's feedback. #

Total comments: 12

Patch Set 9 : Addressed more comments. #

Total comments: 7

Patch Set 10 : Addressed a few more comments. #

Total comments: 18

Patch Set 11 : Addressed snoack's comments. #

Total comments: 2

Patch Set 12 : Addressed more of snoack's comments. #

Total comments: 8

Patch Set 13 : Wladimir's interface + preference file changes for compatability with conversion script. #

Patch Set 14 : Remove includes/common.tmpl which is no longer required. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+636 lines, -3400 lines) Patch
A filters/get_subscriptions.py View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +42 lines, -0 lines 0 comments Download
A filters/parse_interface.py View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +64 lines, -0 lines 0 comments Download
A filters/subscription_sort.py View 1 2 3 4 5 6 7 8 9 1 chunk +30 lines, -0 lines 0 comments Download
M includes/index.tmpl View 1 2 3 1 chunk +16 lines, -0 lines 0 comments Download
A includes/interface.tmpl View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +95 lines, -0 lines 0 comments Download
A includes/preftable.tmpl View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +51 lines, -0 lines 0 comments Download
A includes/subscriptionList.tmpl View 1 2 3 4 5 6 7 8 9 10 1 chunk +89 lines, -0 lines 0 comments Download
D locales/de/known-issues.json View 1 chunk +0 lines, -260 lines 0 comments Download
D locales/de/known-issues-chrome.json View 1 chunk +0 lines, -62 lines 0 comments Download
D locales/de/known-issues-internet-explorer.json View 1 chunk +0 lines, -14 lines 0 comments Download
D locales/en/known-issues.json View 1 chunk +0 lines, -263 lines 0 comments Download
D locales/en/known-issues-chrome.json View 1 chunk +0 lines, -62 lines 0 comments Download
D locales/en/known-issues-internet-explorer.json View 1 chunk +0 lines, -14 lines 0 comments Download
D locales/es/known-issues.json View 1 chunk +0 lines, -248 lines 0 comments Download
D locales/es/known-issues-chrome.json View 1 chunk +0 lines, -62 lines 0 comments Download
D locales/fr/known-issues.json View 1 chunk +0 lines, -248 lines 0 comments Download
D locales/fr/known-issues-chrome.json View 1 chunk +0 lines, -62 lines 0 comments Download
D locales/he/known-issues-chrome.json View 1 chunk +0 lines, -44 lines 0 comments Download
D locales/ko/known-issues.json View 1 chunk +0 lines, -248 lines 0 comments Download
D locales/ko/known-issues-chrome.json View 1 chunk +0 lines, -44 lines 0 comments Download
D locales/lt/known-issues.json View 1 chunk +0 lines, -248 lines 0 comments Download
D locales/nl/known-issues.json View 1 chunk +0 lines, -248 lines 0 comments Download
D locales/nl/known-issues-chrome.json View 1 chunk +0 lines, -62 lines 0 comments Download
D locales/ru/known-issues.json View 1 chunk +0 lines, -206 lines 0 comments Download
D locales/sk/known-issues-chrome.json View 1 chunk +0 lines, -62 lines 0 comments Download
D locales/sk/known-issues-internet-explorer.json View 1 chunk +0 lines, -14 lines 0 comments Download
D locales/zh_CN/known-issues.json View 1 chunk +0 lines, -263 lines 0 comments Download
D locales/zh_CN/known-issues-chrome.json View 1 chunk +0 lines, -62 lines 0 comments Download
D locales/zh_CN/known-issues-internet-explorer.json View 1 chunk +0 lines, -14 lines 0 comments Download
D locales/zh_TW/known-issues.json View 1 chunk +0 lines, -248 lines 0 comments Download
D locales/zh_TW/known-issues-chrome.json View 1 chunk +0 lines, -62 lines 0 comments Download
M pages/index.tmpl View 1 2 3 1 chunk +16 lines, -0 lines 0 comments Download
D pages/known-issues.raw View 1 chunk +0 lines, -171 lines 0 comments Download
D pages/known-issues-chrome.raw View 1 chunk +0 lines, -32 lines 0 comments Download
D pages/known-issues-internet-explorer.raw View 1 chunk +0 lines, -6 lines 0 comments Download
A + pages/maxthon.md View 1 2 3 0 chunks +-1 lines, --1 lines 0 comments Download
A + pages/safari.md View 1 2 3 0 chunks +-1 lines, --1 lines 0 comments Download
A + pages/yandex-browser.md View 1 2 0 chunks +-1 lines, --1 lines 0 comments Download
M settings.ini View 1 2 3 1 chunk +3 lines, -0 lines 0 comments Download
M static/css/index.css View 1 2 3 16 chunks +162 lines, -58 lines 0 comments Download
M static/css/index-desktop.css View 1 2 3 4 chunks +25 lines, -4 lines 0 comments Download
M static/css/index-mobile.css View 1 2 3 2 chunks +4 lines, -2 lines 0 comments Download
M static/css/main.css View 1 chunk +0 lines, -5 lines 0 comments Download
A static/favicon.ico View Binary file 0 comments Download
A static/fonts/SourceSansProBlack.woff View Binary file 0 comments Download
A static/fonts/SourceSansProLight.woff View Binary file 0 comments Download
A static/img/maxthon-instruction.png View 1 2 3 4 5 6 7 8 9 10 11 12 Binary file 0 comments Download
A static/img/maxthon-teaser.png View 1 2 Binary file 0 comments Download
M static/img/sprite-index.png View 1 2 3 Binary file 0 comments Download
M templates/default.tmpl View 1 2 3 4 5 6 7 8 6 chunks +25 lines, -5 lines 0 comments Download
M templates/minimal.tmpl View 1 2 3 1 chunk +17 lines, -0 lines 0 comments Download

Messages

Total messages: 41
kzar
Sept. 17, 2014, 8:35 a.m. (2014-09-17 08:35:05 UTC) #1
Wladimir Palant
Only managed to review the first three files today, should be at least something to ...
Sept. 18, 2014, 8:33 p.m. (2014-09-18 20:33:04 UTC) #2
kzar
Cheers for the feedback :) http://codereview.adblockplus.org/5636796054503424/diff/5629499534213120/filters/parse_interface.py File filters/parse_interface.py (right): http://codereview.adblockplus.org/5636796054503424/diff/5629499534213120/filters/parse_interface.py#newcode3 filters/parse_interface.py:3: def parse_interface(interfaces): On 2014/09/18 ...
Sept. 19, 2014, 11:28 a.m. (2014-09-19 11:28:06 UTC) #3
kzar
chained* On 2014/09/19 11:28:06, kzar wrote: > Cheers for the feedback :) > > http://codereview.adblockplus.org/5636796054503424/diff/5629499534213120/filters/parse_interface.py ...
Sept. 19, 2014, 9:08 p.m. (2014-09-19 21:08:03 UTC) #4
Felix Dahlke
I'll review this soon. Dave, can you add Manvel as a reviewer too? He's the ...
Oct. 6, 2014, 12:23 p.m. (2014-10-06 12:23:31 UTC) #5
kzar
On 2014/10/06 12:23:31, Felix H. Dahlke wrote: > I'll review this soon. Dave, can you ...
Oct. 6, 2014, 12:25 p.m. (2014-10-06 12:25:25 UTC) #6
Sebastian Noack
I also realized that some recent changes (like the Maxthon landing page) aren't included yet. ...
Feb. 13, 2015, 5:51 p.m. (2015-02-13 17:51:48 UTC) #7
Wladimir Palant
On 2015/02/13 17:51:48, Sebastian Noack wrote: > I also realized that some recent changes (like ...
Feb. 13, 2015, 6:27 p.m. (2015-02-13 18:27:56 UTC) #8
Wladimir Palant
On 2015/02/13 18:27:56, Wladimir Palant wrote: > This shouldn't matter - ideally running the convert.py ...
Feb. 13, 2015, 6:31 p.m. (2015-02-13 18:31:40 UTC) #9
kzar
http://codereview.adblockplus.org/5636796054503424/diff/5730192894984192/filters/parse_interface.py File filters/parse_interface.py (right): http://codereview.adblockplus.org/5636796054503424/diff/5730192894984192/filters/parse_interface.py#newcode1 filters/parse_interface.py:1: import re, warnings On 2015/02/13 17:51:49, Sebastian Noack wrote: ...
Feb. 20, 2015, 2:55 p.m. (2015-02-20 14:55:17 UTC) #10
Sebastian Noack
http://codereview.adblockplus.org/5636796054503424/diff/5730192894984192/includes/interface.tmpl File includes/interface.tmpl (right): http://codereview.adblockplus.org/5636796054503424/diff/5730192894984192/includes/interface.tmpl#newcode4 includes/interface.tmpl:4: {{open |safe}}{{ key_name |translate(None, links[key_name])|unescape|safe }}{{close |safe}} On 2015/02/20 ...
Feb. 20, 2015, 3:32 p.m. (2015-02-20 15:32:59 UTC) #11
kzar
http://codereview.adblockplus.org/5636796054503424/diff/5730192894984192/filters/unescape.py File filters/unescape.py (right): http://codereview.adblockplus.org/5636796054503424/diff/5730192894984192/filters/unescape.py#newcode1 filters/unescape.py:1: def unescape(markup): On 2015/02/20 14:55:18, kzar wrote: > On ...
Feb. 20, 2015, 5:04 p.m. (2015-02-20 17:04:47 UTC) #12
kzar
Feb. 20, 2015, 5:38 p.m. (2015-02-20 17:38:13 UTC) #13
Sebastian Noack
http://codereview.adblockplus.org/5636796054503424/diff/5730192894984192/filters/unescape.py File filters/unescape.py (right): http://codereview.adblockplus.org/5636796054503424/diff/5730192894984192/filters/unescape.py#newcode1 filters/unescape.py:1: def unescape(markup): On 2015/02/20 17:04:48, kzar wrote: > On ...
Feb. 20, 2015, 5:49 p.m. (2015-02-20 17:49:38 UTC) #14
kzar
http://codereview.adblockplus.org/5636796054503424/diff/5730192894984192/filters/unescape.py File filters/unescape.py (right): http://codereview.adblockplus.org/5636796054503424/diff/5730192894984192/filters/unescape.py#newcode1 filters/unescape.py:1: def unescape(markup): On 2015/02/20 17:49:38, Sebastian Noack wrote: > ...
Feb. 20, 2015, 6 p.m. (2015-02-20 18:00:43 UTC) #15
kzar
OK so it looks like I already solved the problem of passing links into the ...
Feb. 23, 2015, 7:01 p.m. (2015-02-23 19:01:58 UTC) #16
kzar
Feb. 23, 2015, 7:44 p.m. (2015-02-23 19:44:12 UTC) #17
Sebastian Noack
LGTM
Feb. 23, 2015, 7:51 p.m. (2015-02-23 19:51:10 UTC) #18
Wladimir Palant
http://codereview.adblockplus.org/5636796054503424/diff/5667766619078656/filters/parse_interface.py File filters/parse_interface.py (right): http://codereview.adblockplus.org/5636796054503424/diff/5667766619078656/filters/parse_interface.py#newcode34 filters/parse_interface.py:34: property_version = "" Nit: how about: property_version = value.get("version", ...
Feb. 24, 2015, 8:19 p.m. (2015-02-24 20:19:40 UTC) #19
kzar
Addressed palant's feedback. http://codereview.adblockplus.org/5636796054503424/diff/5667766619078656/filters/parse_interface.py File filters/parse_interface.py (right): http://codereview.adblockplus.org/5636796054503424/diff/5667766619078656/filters/parse_interface.py#newcode34 filters/parse_interface.py:34: property_version = "" On 2015/02/24 20:19:40, ...
Feb. 26, 2015, 7:50 p.m. (2015-02-26 19:50:16 UTC) #20
Wladimir Palant
http://codereview.adblockplus.org/5636796054503424/diff/5667766619078656/includes/interface.tmpl File includes/interface.tmpl (right): http://codereview.adblockplus.org/5636796054503424/diff/5667766619078656/includes/interface.tmpl#newcode26 includes/interface.tmpl:26: {{open|safe}}{{ current_key_name|translate(None, links[key_name]) }}{{close|safe}} On 2015/02/26 19:50:17, kzar wrote: ...
Feb. 26, 2015, 8:35 p.m. (2015-02-26 20:35:03 UTC) #21
kzar
Addressed more comments. http://codereview.adblockplus.org/5636796054503424/diff/5667766619078656/includes/interface.tmpl File includes/interface.tmpl (right): http://codereview.adblockplus.org/5636796054503424/diff/5667766619078656/includes/interface.tmpl#newcode84 includes/interface.tmpl:84: <dd>{{ property.version }} and higher</dd> On ...
Feb. 26, 2015, 9:49 p.m. (2015-02-26 21:49:13 UTC) #22
Wladimir Palant
http://codereview.adblockplus.org/5636796054503424/diff/5164320753188864/filters/get_subscriptions.py File filters/get_subscriptions.py (right): http://codereview.adblockplus.org/5636796054503424/diff/5164320753188864/filters/get_subscriptions.py#newcode37 filters/get_subscriptions.py:37: return list(result.itervalues()) return result.values()? http://codereview.adblockplus.org/5636796054503424/diff/5164320753188864/filters/subscription_sort.py File filters/subscription_sort.py (right): http://codereview.adblockplus.org/5636796054503424/diff/5164320753188864/filters/subscription_sort.py#newcode19 ...
Feb. 26, 2015, 10:01 p.m. (2015-02-26 22:01:46 UTC) #23
kzar
Addressed a few more comments. http://codereview.adblockplus.org/5636796054503424/diff/5164320753188864/filters/get_subscriptions.py File filters/get_subscriptions.py (right): http://codereview.adblockplus.org/5636796054503424/diff/5164320753188864/filters/get_subscriptions.py#newcode37 filters/get_subscriptions.py:37: return list(result.itervalues()) On 2015/02/26 ...
Feb. 26, 2015, 10:12 p.m. (2015-02-26 22:12:16 UTC) #24
Wladimir Palant
LGTM I'll need to take a proper look at the results of the conversion script ...
Feb. 26, 2015, 10:15 p.m. (2015-02-26 22:15:34 UTC) #25
Sebastian Noack
http://codereview.adblockplus.org/5636796054503424/diff/5719887657828352/filters/get_subscriptions.py File filters/get_subscriptions.py (right): http://codereview.adblockplus.org/5636796054503424/diff/5719887657828352/filters/get_subscriptions.py#newcode22 filters/get_subscriptions.py:22: def get_subscriptions(_): I feel that we begin to misuse ...
Feb. 27, 2015, 11:10 a.m. (2015-02-27 11:10:48 UTC) #26
kzar
Addressed snoack's comments. http://codereview.adblockplus.org/5636796054503424/diff/5719887657828352/filters/get_subscriptions.py File filters/get_subscriptions.py (right): http://codereview.adblockplus.org/5636796054503424/diff/5719887657828352/filters/get_subscriptions.py#newcode24 filters/get_subscriptions.py:24: source = urllib.urlopen("https://hg.adblockplus.org/subscriptionlist/archive/default.tar.gz") On 2015/02/27 11:10:49, ...
Feb. 27, 2015, 2:21 p.m. (2015-02-27 14:21:07 UTC) #27
Sebastian Noack
http://codereview.adblockplus.org/5636796054503424/diff/5719887657828352/filters/get_subscriptions.py File filters/get_subscriptions.py (right): http://codereview.adblockplus.org/5636796054503424/diff/5719887657828352/filters/get_subscriptions.py#newcode22 filters/get_subscriptions.py:22: def get_subscriptions(_): On 2015/02/27 11:10:49, Sebastian Noack wrote: > ...
Feb. 27, 2015, 2:42 p.m. (2015-02-27 14:42:45 UTC) #28
kzar
Addressed more of snoack's comments. http://codereview.adblockplus.org/5636796054503424/diff/5719887657828352/includes/preftable.tmpl File includes/preftable.tmpl (right): http://codereview.adblockplus.org/5636796054503424/diff/5719887657828352/includes/preftable.tmpl#newcode40 includes/preftable.tmpl:40: {% elif preference.default in ...
Feb. 27, 2015, 2:57 p.m. (2015-02-27 14:57:10 UTC) #29
Sebastian Noack
LGTM. But still, using a filter when you actually need a function isn't great.
Feb. 27, 2015, 3:03 p.m. (2015-02-27 15:03:42 UTC) #30
Wladimir Palant
I don't care enough about the splitext() issue mentioned above so LGTM regardless. I might ...
Feb. 27, 2015, 3:37 p.m. (2015-02-27 15:37:01 UTC) #31
Sebastian Noack
http://codereview.adblockplus.org/5636796054503424/diff/5719887657828352/filters/get_subscriptions.py File filters/get_subscriptions.py (right): http://codereview.adblockplus.org/5636796054503424/diff/5719887657828352/filters/get_subscriptions.py#newcode22 filters/get_subscriptions.py:22: def get_subscriptions(_): On 2015/02/27 15:37:02, Wladimir Palant wrote: > ...
Feb. 27, 2015, 3:40 p.m. (2015-02-27 15:40:43 UTC) #32
Sebastian Noack
http://codereview.adblockplus.org/5636796054503424/diff/5750345418407936/filters/get_subscriptions.py File filters/get_subscriptions.py (right): http://codereview.adblockplus.org/5636796054503424/diff/5750345418407936/filters/get_subscriptions.py#newcode30 filters/get_subscriptions.py:30: if os.path.splitext(fileinfo.name)[1] != ".subscription": On 2015/02/27 15:40:43, Sebastian Noack ...
Feb. 27, 2015, 3:41 p.m. (2015-02-27 15:41:18 UTC) #33
Wladimir Palant
http://codereview.adblockplus.org/5636796054503424/diff/5750345418407936/filters/get_subscriptions.py File filters/get_subscriptions.py (right): http://codereview.adblockplus.org/5636796054503424/diff/5750345418407936/filters/get_subscriptions.py#newcode30 filters/get_subscriptions.py:30: if os.path.splitext(fileinfo.name)[1] != ".subscription": On 2015/02/27 15:40:43, Sebastian Noack ...
Feb. 27, 2015, 4:19 p.m. (2015-02-27 16:19:57 UTC) #34
Sebastian Noack
http://codereview.adblockplus.org/5636796054503424/diff/5750345418407936/filters/get_subscriptions.py File filters/get_subscriptions.py (right): http://codereview.adblockplus.org/5636796054503424/diff/5750345418407936/filters/get_subscriptions.py#newcode30 filters/get_subscriptions.py:30: if os.path.splitext(fileinfo.name)[1] != ".subscription": On 2015/02/27 16:19:58, Wladimir Palant ...
Feb. 27, 2015, 4:32 p.m. (2015-02-27 16:32:32 UTC) #35
kzar
On 2015/02/27 16:32:32, Sebastian Noack wrote: > http://codereview.adblockplus.org/5636796054503424/diff/5750345418407936/filters/get_subscriptions.py > File filters/get_subscriptions.py (right): > > http://codereview.adblockplus.org/5636796054503424/diff/5750345418407936/filters/get_subscriptions.py#newcode30 ...
Feb. 27, 2015, 4:40 p.m. (2015-02-27 16:40:11 UTC) #36
Wladimir Palant
As explained below, I made some incompatible changes to the converter script. I'll send you ...
March 6, 2015, 11:21 p.m. (2015-03-06 23:21:06 UTC) #37
kzar
Patch Set 13 : Wladimir's interface + preference file changes for compatibility with conversion script. ...
March 7, 2015, 6:39 p.m. (2015-03-07 18:39:24 UTC) #38
kzar
Patch Set 14 : Remove includes/common.tmpl which is no longer required.
March 7, 2015, 7:12 p.m. (2015-03-07 19:12:59 UTC) #39
kzar
Your changes LGTM Wladimir. I've mostly left everything alone, just deleted that descriptions macro we ...
March 7, 2015, 7:16 p.m. (2015-03-07 19:16:58 UTC) #40
Wladimir Palant
March 7, 2015, 7:21 p.m. (2015-03-07 19:21:57 UTC) #41
Yes, LGTM. There are a few relatively minor issues left but these affect the
conversion script. I think we can update the repo and improve the conversion
script later.

Powered by Google App Engine
This is Rietveld