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

Issue 29800557: Issue 6559 - Use Map object for known subscriptions (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 6 months ago by Manish Jethani
Modified:
1 year, 6 months ago
Reviewers:
kzar, sergei
Base URL:
https://hg.adblockplus.org/adblockpluscore/
Visibility:
Public.

Description

Note: This will also require changes in adblockplusui.

Patch Set 1 #

Patch Set 2 : Avoid lookup for disabled subscriptions #

Patch Set 3 : Move check for known subscription into removeSubscription #

Total comments: 6
Unified diffs Side-by-side diffs Delta from patch set Stats (+27 lines, -24 lines) Patch
M lib/filterListener.js View 1 1 chunk +3 lines, -3 lines 2 comments Download
M lib/filterStorage.js View 1 2 5 chunks +13 lines, -10 lines 1 comment Download
M lib/subscriptionClasses.js View 3 chunks +7 lines, -6 lines 1 comment Download
M lib/synchronizer.js View 1 2 2 chunks +4 lines, -5 lines 2 comments Download

Messages

Total messages: 5
Manish Jethani
1 year, 6 months ago (2018-06-06 10:51:01 UTC) #1
Manish Jethani
Patch Set 1 Patch Set 2: Avoid lookup for disabled subscriptions Patch Set 3: Move ...
1 year, 6 months ago (2018-06-06 11:04:43 UTC) #2
kzar
https://codereview.adblockplus.org/29800557/diff/29800568/lib/filterListener.js File lib/filterListener.js (right): https://codereview.adblockplus.org/29800557/diff/29800568/lib/filterListener.js#newcode257 lib/filterListener.js:257: if (!subscription.disabled && On 2018/06/06 11:04:42, Manish Jethani wrote: ...
1 year, 6 months ago (2018-06-07 13:52:26 UTC) #3
Manish Jethani
Back to Patch Set 2 https://codereview.adblockplus.org/29800557/diff/29800568/lib/synchronizer.js File lib/synchronizer.js (left): https://codereview.adblockplus.org/29800557/diff/29800568/lib/synchronizer.js#oldcode204 lib/synchronizer.js:204: if (listed) On 2018/06/07 ...
1 year, 6 months ago (2018-06-07 14:29:46 UTC) #4
kzar
1 year, 6 months ago (2018-06-07 14:35:47 UTC) #5
Patch Set 2 LGTM
Sign in to reply to this message.

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