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

Issue 29860589: Issue 6829 - Free filters when subscriptions are empty (Closed)

Created:
Aug. 21, 2018, 9:44 p.m. by Jon Sonesen
Modified:
Jan. 29, 2019, 3:55 p.m.
Reviewers:
Manish Jethani, hub
CC:
kzar
Base URL:
https://hg.adblockplus.org/adblockpluscore/
Visibility:
Public.

Description

Issue 6829 - Free filters when subscriptions are empty

Patch Set 1 #

Total comments: 9

Patch Set 2 : Address PS1 comment #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+6 lines, -0 lines) Patch
M lib/filterStorage.js View 1 1 chunk +5 lines, -0 lines 0 comments Download
M test/filterStorage.js View 1 1 chunk +1 line, -0 lines 1 comment Download

Messages

Total messages: 11
Jon Sonesen
Aug. 21, 2018, 9:44 p.m. (2018-08-21 21:44:47 UTC) #1
Jon Sonesen
Once I saw this method removeSubscriptionFilters which is iterating over all the filters in a ...
Aug. 21, 2018, 9:50 p.m. (2018-08-21 21:50:41 UTC) #2
Manish Jethani
Similar to what we did here: https://github.com/adblockplus/adblockpluscore/commit/5aca7b994bd142bf44da691a98ec30e779eb5447 I think we should add some tests for ...
Aug. 25, 2018, 3:03 p.m. (2018-08-25 15:03:39 UTC) #3
jsonesen
https://codereview.adblockplus.org/29860589/diff/29860590/lib/filterStorage.js File lib/filterStorage.js (right): https://codereview.adblockplus.org/29860589/diff/29860590/lib/filterStorage.js#newcode683 lib/filterStorage.js:683: if (filter.subscriptions.size == 0) On 2018/08/25 15:03:38, Manish Jethani ...
Aug. 25, 2018, 3:18 p.m. (2018-08-25 15:18:46 UTC) #4
jsonesen
https://codereview.adblockplus.org/29860589/diff/29860590/lib/filterStorage.js File lib/filterStorage.js (right): https://codereview.adblockplus.org/29860589/diff/29860590/lib/filterStorage.js#newcode683 lib/filterStorage.js:683: if (filter.subscriptions.size == 0) On 2018/08/25 15:18:46, jsonesen wrote: ...
Aug. 25, 2018, 3:32 p.m. (2018-08-25 15:32:08 UTC) #5
Manish Jethani
https://codereview.adblockplus.org/29860589/diff/29860590/lib/filterStorage.js File lib/filterStorage.js (right): https://codereview.adblockplus.org/29860589/diff/29860590/lib/filterStorage.js#newcode683 lib/filterStorage.js:683: if (filter.subscriptions.size == 0) On 2018/08/25 15:32:08, jsonesen wrote: ...
Aug. 26, 2018, 3:56 p.m. (2018-08-26 15:56:19 UTC) #6
Jon Sonesen
https://codereview.adblockplus.org/29860589/diff/29860590/lib/filterStorage.js File lib/filterStorage.js (right): https://codereview.adblockplus.org/29860589/diff/29860590/lib/filterStorage.js#newcode148 lib/filterStorage.js:148: On 2018/08/25 15:03:38, Manish Jethani wrote: > We should ...
Aug. 27, 2018, 4:28 p.m. (2018-08-27 16:28:03 UTC) #7
Jon Sonesen
https://codereview.adblockplus.org/29860589/diff/29869582/test/filterStorage.js File test/filterStorage.js (right): https://codereview.adblockplus.org/29860589/diff/29869582/test/filterStorage.js#newcode498 test/filterStorage.js:498: test.equal(Filter.knownFilters[filter1.text], undefined); This test is currently a noop (removing ...
Aug. 30, 2018, 5:11 p.m. (2018-08-30 17:11:43 UTC) #8
Manish Jethani
I'm sorry to keep this one waiting. I think this one is really tricky and ...
Sept. 24, 2018, 11:14 a.m. (2018-09-24 11:14:17 UTC) #9
Jon Sonesen
Word, thanks for the follow up I figured we would shelve this for a bit. ...
Sept. 24, 2018, 3:04 p.m. (2018-09-24 15:04:15 UTC) #10
Jon Sonesen
Jan. 29, 2019, 3:55 p.m. (2019-01-29 15:55:39 UTC) #11
Message was sent while issue was closed.
Closing. 

This ticket was postponed and by now I would like to use gitlab

Powered by Google App Engine
This is Rietveld