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

Issue 29336735: Issue 394 - hit statistics tool data collection (core) (Closed)

Created:
Feb. 19, 2016, 5:26 p.m. by saroyanm
Modified:
Dec. 21, 2017, 10:47 a.m.
Reviewers:
Wladimir Palant
CC:
kzar, Thomas Greiner
Visibility:
Public.

Description

This is dependency review for core changes of filter hit statistics initial implementation: https://codereview.adblockplus.org/6337686776315904/

Patch Set 1 #

Total comments: 6

Patch Set 2 : Include modules conditionally update Downloader to accept callback for download event #

Total comments: 3

Patch Set 3 : Rebase to changeset 593 #

Patch Set 4 : Use Downloader to send the data to server #

Unified diffs Side-by-side diffs Delta from patch set Stats (+42 lines, -4 lines) Patch
M lib/downloader.js View 1 2 3 4 chunks +24 lines, -3 lines 0 comments Download
M lib/filterListener.js View 1 2 2 chunks +8 lines, -0 lines 0 comments Download
M lib/filterStorage.js View 1 2 chunks +10 lines, -1 line 0 comments Download

Messages

Total messages: 7
saroyanm
Feb. 19, 2016, 5:34 p.m. (2016-02-19 17:34:20 UTC) #1
saroyanm
https://codereview.adblockplus.org/29336735/diff/29336736/lib/filterStorage.js File lib/filterStorage.js (right): https://codereview.adblockplus.org/29336735/diff/29336736/lib/filterStorage.js#newcode336 lib/filterStorage.js:336: if (Prefs.sendstats) Hmm, I think it's make sense to ...
Feb. 19, 2016, 6:31 p.m. (2016-02-19 18:31:35 UTC) #2
Wladimir Palant
https://codereview.adblockplus.org/29336735/diff/29336736/lib/filterListener.js File lib/filterListener.js (right): https://codereview.adblockplus.org/29336735/diff/29336736/lib/filterListener.js#newcode33 lib/filterListener.js:33: let {FilterHits} = require("filterHits"); This needs to be included ...
Feb. 29, 2016, 2:40 p.m. (2016-02-29 14:40:31 UTC) #3
saroyanm
https://codereview.adblockplus.org/29336735/diff/29336736/lib/filterListener.js File lib/filterListener.js (right): https://codereview.adblockplus.org/29336735/diff/29336736/lib/filterListener.js#newcode33 lib/filterListener.js:33: let {FilterHits} = require("filterHits"); On 2016/02/29 14:40:30, Wladimir Palant ...
March 18, 2016, 6:24 p.m. (2016-03-18 18:24:48 UTC) #4
Wladimir Palant
https://codereview.adblockplus.org/29336735/diff/29338570/lib/downloader.js File lib/downloader.js (right): https://codereview.adblockplus.org/29336735/diff/29338570/lib/downloader.js#newcode37 lib/downloader.js:37: let Downloader = exports.Downloader = function Downloader(dataSource, initialDelay, checkInterval, ...
March 23, 2016, 11:06 a.m. (2016-03-23 11:06:02 UTC) #5
saroyanm
Wladimir can you please have another look. https://codereview.adblockplus.org/29336735/diff/29338570/lib/downloader.js File lib/downloader.js (right): https://codereview.adblockplus.org/29336735/diff/29338570/lib/downloader.js#newcode37 lib/downloader.js:37: let Downloader ...
April 6, 2016, 3:12 p.m. (2016-04-06 15:12:49 UTC) #6
Wladimir Palant
Dec. 21, 2017, 10:47 a.m. (2017-12-21 10:47:43 UTC) #7
I'm pretty sure that this is no longer relevant, closing.

Powered by Google App Engine
This is Rietveld