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

Issue 29370999: Issue 4783 - Use modern JavaScript syntax for the messageResponder (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 7 months ago by kzar
Modified:
2 years, 7 months ago
Visibility:
Public.

Description

Issue 4783 - Use modern JavaScript syntax for the messageResponder

Patch Set 1 #

Total comments: 5

Patch Set 2 : Remove pointless IIFE #

Patch Set 3 : "use strict"; #

Total comments: 14

Patch Set 4 : Addressed feedback #

Total comments: 2

Patch Set 5 : Use destructuring + const, fix typo #

Total comments: 6

Patch Set 6 : Addressed further feedback #

Unified diffs Side-by-side diffs Delta from patch set Stats (+73 lines, -83 lines) Patch
M messageResponder.js View 1 2 3 4 5 23 chunks +73 lines, -83 lines 0 comments Download

Messages

Total messages: 22
kzar
Patch Set 1 Since the main change for #4579 added the use of some ES6 ...
2 years, 7 months ago (2017-01-11 08:50:44 UTC) #1
Sebastian Noack
https://codereview.adblockplus.org/29370999/diff/29371000/messageResponder.js File messageResponder.js (right): https://codereview.adblockplus.org/29370999/diff/29371000/messageResponder.js#newcode18 messageResponder.js:18: (global => Do we even need an IIFE with ...
2 years, 7 months ago (2017-01-11 16:29:09 UTC) #2
kzar
Patch Set 2 : Remove pointless IIFE https://codereview.adblockplus.org/29370999/diff/29371000/messageResponder.js File messageResponder.js (right): https://codereview.adblockplus.org/29370999/diff/29371000/messageResponder.js#newcode18 messageResponder.js:18: (global => ...
2 years, 7 months ago (2017-01-13 08:44:49 UTC) #3
Sebastian Noack
https://codereview.adblockplus.org/29370999/diff/29371000/messageResponder.js File messageResponder.js (right): https://codereview.adblockplus.org/29370999/diff/29371000/messageResponder.js#newcode18 messageResponder.js:18: (global => On 2017/01/13 08:44:49, kzar wrote: > On ...
2 years, 7 months ago (2017-01-13 11:54:48 UTC) #4
Sebastian Noack
Also, perhaps we should modernize the JavaScript in the rest of adblockplusui in the same ...
2 years, 7 months ago (2017-01-13 12:13:50 UTC) #5
kzar
Patch Set 3 : "use strict"; > Also, perhaps we should modernize the JavaScript in ...
2 years, 7 months ago (2017-01-16 04:41:49 UTC) #6
Sebastian Noack
LGTM
2 years, 7 months ago (2017-01-16 13:49:18 UTC) #7
Thomas Greiner
https://codereview.adblockplus.org/29370999/diff/29371943/messageResponder.js File messageResponder.js (right): https://codereview.adblockplus.org/29370999/diff/29371943/messageResponder.js#newcode22 messageResponder.js:22: var ext = require("ext_background"); Detail: It'd be great if ...
2 years, 7 months ago (2017-01-17 11:03:51 UTC) #8
kzar
Patch Set 4 : Addressed feedback (I didn't test these latest changes yet, will do ...
2 years, 7 months ago (2017-01-17 11:25:29 UTC) #9
kzar
(Sorry I uploaded the wrong diff there, deleted that patchset and uploaded it again!)
2 years, 7 months ago (2017-01-17 11:27:42 UTC) #10
Sebastian Noack
https://codereview.adblockplus.org/29370999/diff/29371943/messageResponder.js File messageResponder.js (right): https://codereview.adblockplus.org/29370999/diff/29371943/messageResponder.js#newcode24 messageResponder.js:24: let port = require("messaging").port; On 2017/01/17 11:25:29, kzar wrote: ...
2 years, 7 months ago (2017-01-17 15:37:00 UTC) #11
Thomas Greiner
https://codereview.adblockplus.org/29370999/diff/29371943/messageResponder.js File messageResponder.js (right): https://codereview.adblockplus.org/29370999/diff/29371943/messageResponder.js#newcode24 messageResponder.js:24: let port = require("messaging").port; On 2017/01/17 15:37:00, Sebastian Noack ...
2 years, 7 months ago (2017-01-17 15:48:40 UTC) #12
Sebastian Noack
https://codereview.adblockplus.org/29370999/diff/29371943/messageResponder.js File messageResponder.js (right): https://codereview.adblockplus.org/29370999/diff/29371943/messageResponder.js#newcode24 messageResponder.js:24: let port = require("messaging").port; On 2017/01/17 15:48:40, Thomas Greiner ...
2 years, 7 months ago (2017-01-17 17:15:49 UTC) #13
Thomas Greiner
https://codereview.adblockplus.org/29370999/diff/29371943/messageResponder.js File messageResponder.js (right): https://codereview.adblockplus.org/29370999/diff/29371943/messageResponder.js#newcode24 messageResponder.js:24: let port = require("messaging").port; On 2017/01/17 17:15:48, Sebastian Noack ...
2 years, 7 months ago (2017-01-17 17:20:09 UTC) #14
Sebastian Noack
On 2017/01/17 17:20:09, Thomas Greiner wrote: > https://codereview.adblockplus.org/29370999/diff/29371943/messageResponder.js > File messageResponder.js (right): > > https://codereview.adblockplus.org/29370999/diff/29371943/messageResponder.js#newcode24 ...
2 years, 7 months ago (2017-01-17 17:27:06 UTC) #15
Sebastian Noack
https://codereview.adblockplus.org/29370999/diff/29371943/messageResponder.js File messageResponder.js (right): https://codereview.adblockplus.org/29370999/diff/29371943/messageResponder.js#newcode24 messageResponder.js:24: let port = require("messaging").port; On 2017/01/17 17:20:08, Thomas Greiner ...
2 years, 7 months ago (2017-01-17 17:30:16 UTC) #16
kzar
Patch Set 5 : Use destructuring + const, fix typo https://codereview.adblockplus.org/29370999/diff/29371943/messageResponder.js File messageResponder.js (right): https://codereview.adblockplus.org/29370999/diff/29371943/messageResponder.js#newcode24 ...
2 years, 7 months ago (2017-01-18 05:50:33 UTC) #17
Sebastian Noack
LGTM
2 years, 7 months ago (2017-01-18 10:05:11 UTC) #18
Thomas Greiner
https://codereview.adblockplus.org/29370999/diff/29372263/messageResponder.js File messageResponder.js (right): https://codereview.adblockplus.org/29370999/diff/29372263/messageResponder.js#newcode30 messageResponder.js:30: const {NotificationStorage} = require("notification"); This doesn't work. What you'd ...
2 years, 7 months ago (2017-01-18 11:40:10 UTC) #19
kzar
Patch Set 6 : Addressed further feedback https://codereview.adblockplus.org/29370999/diff/29372263/messageResponder.js File messageResponder.js (right): https://codereview.adblockplus.org/29370999/diff/29372263/messageResponder.js#newcode30 messageResponder.js:30: const {NotificationStorage} ...
2 years, 7 months ago (2017-01-18 11:54:42 UTC) #20
Thomas Greiner
LGTM
2 years, 7 months ago (2017-01-18 12:18:26 UTC) #21
Sebastian Noack
2 years, 7 months ago (2017-01-18 12:21:37 UTC) #22
LGTM
Sign in to reply to this message.

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