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

Issue 29433625: Issue 5220 - Update the IO API (Closed)

Created:
May 8, 2017, 2:14 p.m. by hub
Modified:
May 23, 2017, 1:15 p.m.
Base URL:
https://hg.adblockplus.org/adblockpluschrome/
Visibility:
Public.

Description

Issue 5220 - Update the IO API

Patch Set 1 #

Total comments: 20

Patch Set 2 : Fix the IO implementation based on feedback. #

Total comments: 10

Patch Set 3 : Fixes to the promise flow and review comments. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+123 lines, -91 lines) Patch
M dependencies View 1 chunk +1 line, -1 line 0 comments Download
M lib/compat.js View 1 2 1 chunk +0 lines, -33 lines 0 comments Download
M lib/io.js View 1 2 1 chunk +122 lines, -57 lines 0 comments Download

Messages

Total messages: 9
hub
May 8, 2017, 2:14 p.m. (2017-05-08 14:14:27 UTC) #1
Wladimir Palant
I added Sebastian to CC - it's his module, he should be aware of this ...
May 9, 2017, 9:12 a.m. (2017-05-09 09:12:28 UTC) #2
hub
Update patch to fix these. https://codereview.adblockplus.org/29433625/diff/29433626/lib/io.js File lib/io.js (right): https://codereview.adblockplus.org/29433625/diff/29433626/lib/io.js#newcode26 lib/io.js:26: return keyPrefix + (file ...
May 10, 2017, 4:08 p.m. (2017-05-10 16:08:51 UTC) #3
Sebastian Noack
https://codereview.adblockplus.org/29433625/diff/29433626/lib/io.js File lib/io.js (right): https://codereview.adblockplus.org/29433625/diff/29433626/lib/io.js#newcode49 lib/io.js:49: return new Promise((resolve) => On 2017/05/10 16:08:51, hub wrote: ...
May 22, 2017, 1:36 p.m. (2017-05-22 13:36:30 UTC) #4
Wladimir Palant
FakeFile implementation hasn't been removed with the current patch. https://codereview.adblockplus.org/29433625/diff/29435642/lib/io.js File lib/io.js (right): https://codereview.adblockplus.org/29433625/diff/29435642/lib/io.js#newcode73 lib/io.js:73: ...
May 22, 2017, 1:46 p.m. (2017-05-22 13:46:32 UTC) #5
Sebastian Noack
https://codereview.adblockplus.org/29433625/diff/29435642/lib/io.js File lib/io.js (right): https://codereview.adblockplus.org/29433625/diff/29435642/lib/io.js#newcode73 lib/io.js:73: * Name of the file to be read On ...
May 22, 2017, 1:49 p.m. (2017-05-22 13:49:53 UTC) #6
kzar
https://codereview.adblockplus.org/29433625/diff/29435642/lib/io.js File lib/io.js (right): https://codereview.adblockplus.org/29433625/diff/29435642/lib/io.js#newcode73 lib/io.js:73: * Name of the file to be read On ...
May 22, 2017, 2 p.m. (2017-05-22 14:00:41 UTC) #7
hub
Updated patch, addressed comments, fixed some Promise code that was wrong. https://codereview.adblockplus.org/29433625/diff/29435642/lib/io.js File lib/io.js (right): ...
May 23, 2017, 12:31 p.m. (2017-05-23 12:31:50 UTC) #8
Sebastian Noack
May 23, 2017, 12:41 p.m. (2017-05-23 12:41:54 UTC) #9
LGTM

Powered by Google App Engine
This is Rietveld