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

Issue 5662732741246976: Issue 2144 - [adblockplus.org Anwiki to CMS migration] get_subscriptions filter expects a local sub… (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 11 months ago by Wladimir Palant
Modified:
4 years, 11 months ago
Reviewers:
kzar, Sebastian Noack
Visibility:
Public.

Description

Issue 2144 - [adblockplus.org Anwiki to CMS migration] get_subscriptions filter expects a local sub…

Patch Set 1 #

Total comments: 5

Patch Set 2 : Addressed comments #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+15 lines, -3 lines) Patch
M filters/get_subscriptions.py View 1 1 chunk +15 lines, -3 lines 2 comments Download

Messages

Total messages: 5
Wladimir Palant
4 years, 11 months ago (2015-03-13 19:44:36 UTC) #1
Sebastian Noack
http://codereview.adblockplus.org/5662732741246976/diff/5629499534213120/filters/get_subscriptions.py File filters/get_subscriptions.py (right): http://codereview.adblockplus.org/5662732741246976/diff/5629499534213120/filters/get_subscriptions.py#newcode23 filters/get_subscriptions.py:23: import sitescripts.subscriptions.subscriptionParser as subscriptionParser Nit: from ... import http://codereview.adblockplus.org/5662732741246976/diff/5629499534213120/filters/get_subscriptions.py#newcode32 ...
4 years, 11 months ago (2015-03-13 19:51:44 UTC) #2
Wladimir Palant
LGTM http://codereview.adblockplus.org/5662732741246976/diff/5629499534213120/filters/get_subscriptions.py File filters/get_subscriptions.py (right): http://codereview.adblockplus.org/5662732741246976/diff/5629499534213120/filters/get_subscriptions.py#newcode23 filters/get_subscriptions.py:23: import sitescripts.subscriptions.subscriptionParser as subscriptionParser On 2015/03/13 19:51:45, Sebastian ...
4 years, 11 months ago (2015-03-13 19:58:51 UTC) #3
Sebastian Noack
LGTM http://codereview.adblockplus.org/5662732741246976/diff/5629499534213120/filters/get_subscriptions.py File filters/get_subscriptions.py (right): http://codereview.adblockplus.org/5662732741246976/diff/5629499534213120/filters/get_subscriptions.py#newcode32 filters/get_subscriptions.py:32: settings.readfp(utf8_reader(urllib.urlopen("https://hg.adblockplus.org/subscriptionlist/rawfile/default/settings"))) On 2015/03/13 19:58:51, Wladimir Palant wrote: > ...
4 years, 11 months ago (2015-03-13 20:06:34 UTC) #4
Wladimir Palant
4 years, 11 months ago (2015-03-13 20:08:54 UTC) #5
http://codereview.adblockplus.org/5662732741246976/diff/5724160613416960/filt...
File filters/get_subscriptions.py (right):

http://codereview.adblockplus.org/5662732741246976/diff/5724160613416960/filt...
filters/get_subscriptions.py:32: settings_handle =
urllib.urlopen("https://hg.adblockplus.org/subscriptionlist/rawfile/default/settings")
On 2015/03/13 20:06:34, Sebastian Noack wrote:
> Consider moving that logic into subscriptionParser where we could keep using
hg,
> just falling back to a remote URL, if there is no local repo, or something
like
> that.

Yes, that's the idea - the hacks here are merely the short-term solution.
Sign in to reply to this message.

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