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

Issue 29796555: Issue 6621

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 weeks, 2 days ago by geo
Modified:
2 days, 15 hours ago
CC:
Felix Dahlke
Visibility:
Public.

Description

Issue 6621 - Use IndexDB for filter storage on Edge

Patch Set 1 #

Total comments: 40

Patch Set 2 : Most of the changes from patch 1 #

Patch Set 3 : Comment improvements and a rename #

Total comments: 19

Patch Set 4 : Most of the changes from P3 #

Total comments: 6

Patch Set 5 : Remaining change from p3 and changes from p4 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+534 lines, -27 lines) Patch
D lib/io.js View 1 2 chunks +1 line, -27 lines 0 comments Download
A lib/ioIndexedDB.js View 1 2 3 4 1 chunk +303 lines, -0 lines 0 comments Download
M metadata.edge View 1 2 3 1 chunk +3 lines, -0 lines 0 comments Download
A qunit/tests/ioIndexedDB.js View 1 1 chunk +227 lines, -0 lines 0 comments Download

Messages

Total messages: 15
geo
2 weeks, 2 days ago (2018-06-01 14:46:16 UTC) #1
kzar
Thanks Geo, I'll take a look through this next week! In the mean time please ...
2 weeks, 2 days ago (2018-06-01 14:53:26 UTC) #2
Sebastian Noack
Hi Geo, thanks for the patch! The overall implementation looks great, but I have some ...
2 weeks, 2 days ago (2018-06-01 19:15:46 UTC) #3
kzar
Good work Geo, my comments are mostly "nits" (small stylistic things). https://codereview.adblockplus.org/29796555/diff/29796556/lib/ioIndexedDB.js File lib/ioIndexedDB.js (right): ...
1 week, 6 days ago (2018-06-04 11:06:08 UTC) #4
kzar
https://codereview.adblockplus.org/29796555/diff/29796556/metadata.edge File metadata.edge (right): https://codereview.adblockplus.org/29796555/diff/29796556/metadata.edge#newcode17 metadata.edge:17: qunit/tests.js += qunit/tests/ioIndexedDB.js On 2018/06/04 11:06:07, kzar wrote: > ...
1 week, 6 days ago (2018-06-04 18:06:56 UTC) #5
geo
Thank you for reviewing my initial code. I've replied to most of the comments. The ...
1 week, 6 days ago (2018-06-05 07:15:56 UTC) #6
kzar
https://codereview.adblockplus.org/29796555/diff/29799557/lib/io.js File lib/io.js (left): https://codereview.adblockplus.org/29796555/diff/29799557/lib/io.js#oldcode85 lib/io.js:85: * Copies a file. Nice, always good to remove ...
1 week, 5 days ago (2018-06-05 12:38:54 UTC) #7
geo
https://codereview.adblockplus.org/29796555/diff/29796556/metadata.edge File metadata.edge (right): https://codereview.adblockplus.org/29796555/diff/29796556/metadata.edge#newcode17 metadata.edge:17: qunit/tests.js += qunit/tests/ioIndexedDB.js On 2018/06/04 18:06:56, kzar wrote: > ...
1 week ago (2018-06-11 07:02:45 UTC) #8
kzar
https://codereview.adblockplus.org/29796555/diff/29799557/lib/ioIndexedDB.js File lib/ioIndexedDB.js (right): https://codereview.adblockplus.org/29796555/diff/29799557/lib/ioIndexedDB.js#newcode50 lib/ioIndexedDB.js:50: return openDB(dbConfig) On 2018/06/11 07:02:45, geo wrote: > On ...
5 days, 22 hours ago (2018-06-12 10:46:17 UTC) #9
Sebastian Noack
https://codereview.adblockplus.org/29796555/diff/29799557/lib/ioIndexedDB.js File lib/ioIndexedDB.js (right): https://codereview.adblockplus.org/29796555/diff/29799557/lib/ioIndexedDB.js#newcode50 lib/ioIndexedDB.js:50: return openDB(dbConfig) On 2018/06/12 10:46:16, kzar wrote: > On ...
5 days, 8 hours ago (2018-06-13 00:55:13 UTC) #10
geo
https://codereview.adblockplus.org/29796555/diff/29799557/lib/ioIndexedDB.js File lib/ioIndexedDB.js (right): https://codereview.adblockplus.org/29796555/diff/29799557/lib/ioIndexedDB.js#newcode50 lib/ioIndexedDB.js:50: return openDB(dbConfig) On 2018/06/13 00:55:11, Sebastian Noack wrote: > ...
3 days, 19 hours ago (2018-06-14 13:41:27 UTC) #11
Oleksandr
On 2018/06/14 13:41:27, geo wrote: > https://codereview.adblockplus.org/29796555/diff/29799557/lib/ioIndexedDB.js > File lib/ioIndexedDB.js (right): > > https://codereview.adblockplus.org/29796555/diff/29799557/lib/ioIndexedDB.js#newcode50 > ...
3 days, 12 hours ago (2018-06-14 20:57:05 UTC) #12
Sebastian Noack
Also LGTM from my side.
3 days, 12 hours ago (2018-06-14 21:05:29 UTC) #13
kzar
LGTM, nice work. Please email a (squashed) patch to Sebastian or myself when you get ...
3 days ago (2018-06-15 08:30:42 UTC) #14
Sebastian Noack
2 days, 15 hours ago (2018-06-15 18:03:30 UTC) #15
I went ahead, downloading the patch from Rietveld and pushing it:
https://hg.adblockplus.org/adblockpluschrome/rev/d97edcab2c18

Thanks for your contribution! Feel free to close this review now.
Sign in to reply to this message.

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