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

Issue 29375899: Issue 4871 - Start using ESLint for adblockplusui (Closed)

Created:
Feb. 20, 2017, 10:01 a.m. by kzar
Modified:
March 16, 2017, 10:50 a.m.
CC:
Felix Dahlke, Sebastian Noack
Visibility:
Public.

Description

Issue 4871 - Start using ESLint for adblockplusui adblockpluschrome still seems to work when using these changes, but I need to go through and test more thoroughly, especially things like the new options page.

Patch Set 1 #

Total comments: 6

Patch Set 2 : Stop using commonjs, fix other problems #

Total comments: 20

Patch Set 3 : Started addressing Sebastian's initial feedback #

Patch Set 4 : Addressed the rest of Sebastian's feedback #

Patch Set 5 : Fix regressions with the new options page #

Total comments: 32

Patch Set 6 : Addressed Thomas' feedback #

Patch Set 7 : Rebased. #

Total comments: 2

Patch Set 8 : Move dots to the start of the line #

Patch Set 9 : Remove the arrow-parens rule #

Total comments: 19

Patch Set 10 : Addressed Wladimir's feedback, restored IIFEs #

Patch Set 11 : Rebased. #

Patch Set 12 : Fix imports in antiadblockInit.js #

Total comments: 3

Patch Set 13 : Removed unused import #

Patch Set 14 : Avoid violating operator-linebreak rule #

Unified diffs Side-by-side diffs Delta from patch set Stats (+811 lines, -757 lines) Patch
A .eslintrc.json View 1 2 3 4 5 6 7 8 9 1 chunk +11 lines, -0 lines 0 comments Download
M README.md View 1 chunk +11 lines, -0 lines 0 comments Download
M background.js View 1 2 3 4 5 6 7 8 9 17 chunks +137 lines, -111 lines 0 comments Download
M common.js View 1 2 3 4 5 1 chunk +110 lines, -112 lines 0 comments Download
M devtools-panel.js View 1 2 3 4 5 10 chunks +37 lines, -30 lines 0 comments Download
M ext/background.js View 1 2 3 4 5 6 7 8 9 6 chunks +22 lines, -20 lines 0 comments Download
M ext/common.js View 1 2 3 4 5 6 7 8 9 8 chunks +50 lines, -48 lines 0 comments Download
M ext/content.js View 1 2 3 4 5 6 7 8 9 2 chunks +27 lines, -21 lines 0 comments Download
M ext/devtools.js View 1 2 3 1 chunk +11 lines, -14 lines 0 comments Download
M firstRun.js View 1 2 3 4 5 6 7 8 9 9 chunks +24 lines, -23 lines 0 comments Download
M i18n.js View 1 2 3 4 5 2 chunks +41 lines, -42 lines 0 comments Download
A lib/.eslintrc.json View 1 2 3 4 5 6 7 8 9 1 chunk +8 lines, -0 lines 0 comments Download
M lib/antiadblockInit.js View 1 2 3 4 5 6 7 8 9 10 11 12 4 chunks +16 lines, -14 lines 0 comments Download
M messageResponder.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 10 chunks +37 lines, -34 lines 0 comments Download
M new-options.js View 1 2 3 4 5 6 7 48 chunks +269 lines, -288 lines 0 comments Download

Messages

Total messages: 25
kzar
Patch Set 1
Feb. 20, 2017, 10:33 a.m. (2017-02-20 10:33:45 UTC) #1
kzar
Patch Set 2 : Stop using commonjs, fix other problems
Feb. 21, 2017, 5:16 a.m. (2017-02-21 05:16:19 UTC) #2
Sebastian Noack
https://codereview.adblockplus.org/29375899/diff/29376653/background.js File background.js (right): https://codereview.adblockplus.org/29375899/diff/29376653/background.js#newcode135 background.js:135: /* eslint-disable object-shorthand */ Note that the object-shorthand rule ...
Feb. 21, 2017, 12:21 p.m. (2017-02-21 12:21:21 UTC) #3
kzar
Patch Set 3 : Started addressing Sebastian's initial feedback Ran out of time, so didn't ...
Feb. 23, 2017, 11:10 a.m. (2017-02-23 11:10:22 UTC) #4
kzar
Patch Set 4 : Addressed the rest of Sebastian's feedback https://codereview.adblockplus.org/29375899/diff/29376653/messageResponder.js File messageResponder.js (right): https://codereview.adblockplus.org/29375899/diff/29376653/messageResponder.js#newcode21 ...
March 1, 2017, 5:36 a.m. (2017-03-01 05:36:21 UTC) #5
kzar
Patch Set 5 : Fix regressions with the new options page
March 1, 2017, 8:26 a.m. (2017-03-01 08:26:25 UTC) #6
Thomas Greiner
https://codereview.adblockplus.org/29375899/diff/29375900/background.js File background.js (right): https://codereview.adblockplus.org/29375899/diff/29375900/background.js#newcode105 background.js:105: Object.keys(prefs).forEach(key => Detail: Mind keeping the brackets? At least ...
March 1, 2017, 5:39 p.m. (2017-03-01 17:39:36 UTC) #7
kzar
(Just replying to a few comments for now.) https://codereview.adblockplus.org/29375899/diff/29375900/background.js File background.js (right): https://codereview.adblockplus.org/29375899/diff/29375900/background.js#newcode105 background.js:105: Object.keys(prefs).forEach(key ...
March 2, 2017, 4:36 a.m. (2017-03-02 04:36:02 UTC) #8
kzar
Patch Set 6 : Addressed Thomas' feedback https://codereview.adblockplus.org/29375899/diff/29375900/background.js File background.js (right): https://codereview.adblockplus.org/29375899/diff/29375900/background.js#newcode105 background.js:105: Object.keys(prefs).forEach(key => ...
March 7, 2017, 12:48 p.m. (2017-03-07 12:48:33 UTC) #9
kzar
Patch Set 7 : Rebased.
March 7, 2017, 12:56 p.m. (2017-03-07 12:56:51 UTC) #10
Thomas Greiner
Only a small question regarding the "max-len" rule and an answer to your question in ...
March 7, 2017, 1:33 p.m. (2017-03-07 13:33:03 UTC) #11
kzar
Patch Set 8 : Move dots to the start of the line Also I should ...
March 8, 2017, 10:29 a.m. (2017-03-08 10:29:33 UTC) #12
Thomas Greiner
LGTM unless you want to take https://issues.adblockplus.org/ticket/3692#comment:51 into consideration but I don't mind either way.
March 8, 2017, 11:32 a.m. (2017-03-08 11:32:42 UTC) #13
kzar
https://codereview.adblockplus.org/29375899/diff/29377713/messageResponder.js File messageResponder.js (right): https://codereview.adblockplus.org/29375899/diff/29377713/messageResponder.js#newcode21 messageResponder.js:21: if (typeof ext == "undefined") I'm a bit confused ...
March 8, 2017, 2:15 p.m. (2017-03-08 14:15:54 UTC) #14
kzar
Patch Set 9 : Remove the arrow-parens rule
March 9, 2017, 10:36 a.m. (2017-03-09 10:36:59 UTC) #15
Wladimir Palant
https://codereview.adblockplus.org/29375899/diff/29379555/.eslintrc.json File .eslintrc.json (right): https://codereview.adblockplus.org/29375899/diff/29379555/.eslintrc.json#newcode11 .eslintrc.json:11: "require": true The way I see it, exports and ...
March 9, 2017, 3:05 p.m. (2017-03-09 15:05:09 UTC) #16
kzar
Patch Set 10 : Addressed Wladimir's feedback, restored IIFEs https://codereview.adblockplus.org/29375899/diff/29379555/.eslintrc.json File .eslintrc.json (right): https://codereview.adblockplus.org/29375899/diff/29379555/.eslintrc.json#newcode11 .eslintrc.json:11: ...
March 10, 2017, 7:29 a.m. (2017-03-10 07:29:01 UTC) #17
kzar
Patch Set 11 : Rebased.
March 14, 2017, 7:36 a.m. (2017-03-14 07:36:59 UTC) #18
kzar
Patch Set 12 : Fix imports in antiadblockInit.js
March 14, 2017, 10:13 a.m. (2017-03-14 10:13:36 UTC) #19
kzar
https://codereview.adblockplus.org/29375899/diff/29383618/lib/antiadblockInit.js File lib/antiadblockInit.js (right): https://codereview.adblockplus.org/29375899/diff/29383618/lib/antiadblockInit.js#newcode20 lib/antiadblockInit.js:20: const {Services} = Cu.import("resource://gre/modules/Services.jsm", {}); I just noticed this ...
March 14, 2017, 10:57 a.m. (2017-03-14 10:57:37 UTC) #20
Wladimir Palant
LGTM regardless of whether you decide to remove the unnecessary Services import with this commit ...
March 14, 2017, 11:14 a.m. (2017-03-14 11:14:58 UTC) #21
kzar
Patch Set 13 : Removed unused import https://codereview.adblockplus.org/29375899/diff/29379555/background.js File background.js (right): https://codereview.adblockplus.org/29375899/diff/29379555/background.js#newcode21 background.js:21: function EventEmitter() ...
March 15, 2017, 3:16 a.m. (2017-03-15 03:16:59 UTC) #22
kzar
Patch Set 14 : Avoid violating operator-linebreak rule
March 15, 2017, 4:44 a.m. (2017-03-15 04:44:48 UTC) #23
Thomas Greiner
LGTM
March 15, 2017, 5:22 p.m. (2017-03-15 17:22:48 UTC) #24
Wladimir Palant
March 16, 2017, 10:50 a.m. (2017-03-16 10:50:26 UTC) #25
Message was sent while issue was closed.
LGTM

Powered by Google App Engine
This is Rietveld