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

Issue 29374674: Issue 4864 - Start using ESLint for adblockpluschrome (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 6 months ago by kzar
Modified:
2 years, 4 months ago
Reviewers:
Sebastian Noack, wspee
CC:
Felix Dahlke, Jon Sonesen, Wladimir Palant, wspee
Visibility:
Public.

Description

Issue 4864 - Start using ESLint for adblockpluschrome

Patch Set 1 #

Patch Set 2 : Reduce inline eslint-disable comments #

Patch Set 3 : Use var for ext declarations again #

Total comments: 39

Patch Set 4 : Addressed some of Sebastian's feedback, made other improvements #

Patch Set 5 : Stop using commonjs #

Patch Set 6 : Rebased. #

Patch Set 7 : Rebased. #

Patch Set 8 : Add arrow-parens rule to match existing convention #

Patch Set 9 : Update Cu.import use to correspond with adblockpluscore changes, removed unused utils.js file #

Patch Set 10 : Remove the arrow-parens rule #

Total comments: 1

Patch Set 11 : Restored IIFEs and chrome/ext/common.js #

Patch Set 12 : Fixed typo with shadowRoot getter #

Total comments: 28

Patch Set 13 : Addressed Wladimir's feedback #

Patch Set 14 : Update adblockpluscore dependency #

Patch Set 15 : Update adblockplusui dependency #

Patch Set 16 : Rebased #

Patch Set 17 : Update adblockplusui dependency to include the notification fix #

Patch Set 18 : Rebased, restored adblockplusui dependency revision #

Total comments: 2

Patch Set 19 : Rebased #

Patch Set 20 : Remove some unnecessary changes #

Patch Set 21 : Remove extra space #

Total comments: 13

Patch Set 22 : Rebased to include Jon's webRequest WebSocket blocking changes #

Patch Set 23 : Address Sebastian's feedback, don't update adblockpluscore #

Total comments: 6

Patch Set 24 : Tidy up hasRecord and some notification logic #

Total comments: 3

Patch Set 25 : Remove two globals from tests #

Patch Set 26 : Addressed more feedback #

Patch Set 27 : Use .includes again #

Unified diffs Side-by-side diffs Delta from patch set Stats (+673 lines, -494 lines) Patch
A .eslintignore View 1 chunk +4 lines, -0 lines 0 comments Download
A .eslintrc.json View 1 2 3 4 8 9 1 chunk +13 lines, -0 lines 0 comments Download
M README.md View 1 2 3 1 chunk +11 lines, -0 lines 0 comments Download
M background.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 1 chunk +1 line, -1 line 0 comments Download
M chrome/devtools.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +1 line, -6 lines 0 comments Download
M chrome/ext/background.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 15 chunks +40 lines, -35 lines 0 comments Download
M chrome/ext/common.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 2 chunks +5 lines, -3 lines 0 comments Download
M chrome/ext/content.js View 1 2 3 4 5 6 7 8 9 10 2 chunks +2 lines, -1 line 0 comments Download
M chrome/ext/devtools.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +2 lines, -1 line 0 comments Download
M chrome/ext/popup.js View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +5 lines, -4 lines 0 comments Download
M composer.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 3 chunks +5 lines, -5 lines 0 comments Download
M composer.postload.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 9 chunks +14 lines, -25 lines 0 comments Download
M ext/background.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 2 chunks +2 lines, -1 line 0 comments Download
M ext/common.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 2 chunks +5 lines, -4 lines 0 comments Download
M include.preload.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 16 chunks +48 lines, -56 lines 0 comments Download
M lib/compat.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 7 chunks +7 lines, -19 lines 0 comments Download
M lib/csp.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 1 chunk +2 lines, -1 line 0 comments Download
M lib/devtools.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 13 chunks +28 lines, -33 lines 0 comments Download
M lib/filterComposer.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 10 chunks +38 lines, -26 lines 0 comments Download
M lib/filterValidation.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 6 chunks +29 lines, -14 lines 0 comments Download
M lib/icon.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 5 chunks +18 lines, -13 lines 0 comments Download
M lib/io.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +2 lines, -0 lines 0 comments Download
M lib/messaging.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 2 chunks +3 lines, -2 lines 0 comments Download
M lib/notificationHelper.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 7 chunks +28 lines, -19 lines 0 comments Download
M lib/popupBlocker.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 3 chunks +3 lines, -3 lines 0 comments Download
M lib/prefs.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 3 chunks +6 lines, -3 lines 0 comments Download
M lib/requestBlocker.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 1 chunk +2 lines, -0 lines 0 comments Download
M lib/subscriptionInit.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +1 line, -1 line 0 comments Download
M lib/tldjs.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 2 chunks +3 lines, -2 lines 0 comments Download
M lib/url.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 2 chunks +3 lines, -4 lines 0 comments Download
M lib/utils.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 3 chunks +7 lines, -2 lines 0 comments Download
M lib/whitelisting.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 5 chunks +5 lines, -2 lines 0 comments Download
M notification.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 1 chunk +8 lines, -9 lines 0 comments Download
M options.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 15 chunks +72 lines, -54 lines 0 comments Download
M popup.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 3 chunks +18 lines, -8 lines 0 comments Download
A qunit/.eslintrc.json View 1 chunk +5 lines, -0 lines 0 comments Download
M qunit/common.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 2 chunks +6 lines, -25 lines 0 comments Download
M qunit/tests/cssEscaping.js View 1 2 3 4 5 6 7 8 9 10 3 chunks +7 lines, -10 lines 0 comments Download
M qunit/tests/filterValidation.js View 1 2 3 4 5 6 7 8 9 10 4 chunks +23 lines, -14 lines 0 comments Download
M qunit/tests/prefs.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 4 chunks +85 lines, -25 lines 0 comments Download
M qunit/tests/url.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 5 chunks +78 lines, -32 lines 0 comments Download
M qunit/tests/versionComparator.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 3 chunks +11 lines, -3 lines 0 comments Download
M stats.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 3 chunks +15 lines, -5 lines 0 comments Download
M subscriptionLink.postload.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +2 lines, -2 lines 0 comments Download
M utils.js View 1 2 3 4 5 6 7 8 1 chunk +0 lines, -21 lines 0 comments Download

Messages

Total messages: 34
kzar
Patch Set 1 Changes as requested, but as mentioned in IRC this is a WIP ...
2 years, 6 months ago (2017-02-07 17:06:17 UTC) #1
kzar
Patch Set 2 : Reduce inline eslint-disable comments
2 years, 6 months ago (2017-02-08 07:29:05 UTC) #2
kzar
Patch Set 3 : Use var for ext declarations again Unfortunately using `let` for the ...
2 years, 6 months ago (2017-02-08 09:04:20 UTC) #3
Sebastian Noack
https://codereview.adblockplus.org/29374674/diff/29374779/.eslintignore File .eslintignore (right): https://codereview.adblockplus.org/29374674/diff/29374779/.eslintignore#newcode1 .eslintignore:1: lib/punycode.js I really don't like how the ESLint configuration ...
2 years, 6 months ago (2017-02-09 01:04:52 UTC) #4
kzar
Patch Set 4 : Addressed some of Sebastian's feedback, made other improvements https://codereview.adblockplus.org/29374674/diff/29374779/.eslintignore File .eslintignore ...
2 years, 6 months ago (2017-02-20 10:27:34 UTC) #5
kzar
Patch Set 5 : Stop using commonjs
2 years, 6 months ago (2017-02-21 05:10:22 UTC) #6
kzar
Patch Set 6 : Rebased. Adding Winsley to Cc since this latest Patch Set merges ...
2 years, 5 months ago (2017-02-28 15:54:59 UTC) #7
wspee
This doesn't break my latest changes. I also looked through the other changes and made ...
2 years, 5 months ago (2017-03-02 10:27:14 UTC) #8
kzar
Patch Set 7 : Rebased. Patch Set 8 : Add arrow-parens rule to match existing ...
2 years, 5 months ago (2017-03-07 11:43:18 UTC) #9
kzar
Patch Set 9 : Update Cu.import use to correspond with adblockpluscore changes, removed unused utils.js ...
2 years, 5 months ago (2017-03-08 12:27:42 UTC) #10
kzar
Patch Set 10 : Remove the arrow-parens rule
2 years, 5 months ago (2017-03-09 10:22:17 UTC) #11
kzar
Any chance you could also have a look through this review Wladimir?
2 years, 5 months ago (2017-03-10 08:29:23 UTC) #12
kzar
Patch Set 11 : Restored IIFEs and chrome/ext/common.js As discussed in the other reviews using ...
2 years, 5 months ago (2017-03-14 08:10:22 UTC) #13
kzar
Patch Set 12 : Fixed typo with shadowRoot getter
2 years, 5 months ago (2017-03-14 10:33:41 UTC) #14
Wladimir Palant
https://codereview.adblockplus.org/29374674/diff/29383722/.eslintrc.json File .eslintrc.json (right): https://codereview.adblockplus.org/29374674/diff/29383722/.eslintrc.json#newcode6 .eslintrc.json:6: "webextensions": true Isn't "browser" already implied by "webextensions"? Did ...
2 years, 5 months ago (2017-03-14 13:03:34 UTC) #15
kzar
Patch Set 13 : Addressed Wladimir's feedback https://codereview.adblockplus.org/29374674/diff/29383722/.eslintrc.json File .eslintrc.json (right): https://codereview.adblockplus.org/29374674/diff/29383722/.eslintrc.json#newcode6 .eslintrc.json:6: "webextensions": true ...
2 years, 5 months ago (2017-03-15 04:57:56 UTC) #16
kzar
Patch Set 14 : Update adblockpluscore dependency
2 years, 5 months ago (2017-03-15 11:45:42 UTC) #17
kzar
Patch Set 15 : Update adblockplusui dependency
2 years, 5 months ago (2017-03-16 08:35:48 UTC) #18
kzar
Patch Set 16 : Rebased
2 years, 5 months ago (2017-03-17 12:46:40 UTC) #19
kzar
Patch Set 17 : Update adblockplusui dependency to include the notification fix
2 years, 5 months ago (2017-03-19 06:52:27 UTC) #20
kzar
Patch Set 18 : Rebased, restored adblockplusui dependency revision
2 years, 5 months ago (2017-03-22 07:22:17 UTC) #21
Sebastian Noack
https://codereview.adblockplus.org/29374674/diff/29391670/dependencies File dependencies (right): https://codereview.adblockplus.org/29374674/diff/29391670/dependencies#newcode6 dependencies:6: adblockplusui = adblockplusui hg:a78bfd4024b7 git:0feaaf7 It seems you did ...
2 years, 5 months ago (2017-03-23 06:38:40 UTC) #22
kzar
Patch Set 19 : Rebased https://codereview.adblockplus.org/29374674/diff/29391670/dependencies File dependencies (right): https://codereview.adblockplus.org/29374674/diff/29391670/dependencies#newcode6 dependencies:6: adblockplusui = adblockplusui hg:a78bfd4024b7 ...
2 years, 5 months ago (2017-03-23 06:51:04 UTC) #23
kzar
Patch Set 20 : Remove some unnecessary changes
2 years, 5 months ago (2017-03-23 09:36:15 UTC) #24
kzar
Patch Set 21 : Remove extra space
2 years, 5 months ago (2017-03-23 10:32:53 UTC) #25
Sebastian Noack
https://codereview.adblockplus.org/29374674/diff/29374779/composer.postload.js File composer.postload.js (right): https://codereview.adblockplus.org/29374674/diff/29374779/composer.postload.js#newcode18 composer.postload.js:18: /* globals checkCollapse, elemhide, getURLsFromElement, typeMap */ On 2017/02/20 ...
2 years, 4 months ago (2017-03-29 17:14:21 UTC) #26
kzar
Patch Set 22 : Rebased to include Jon's webRequest WebSocket blocking changes Patch Set 23 ...
2 years, 4 months ago (2017-03-30 07:20:54 UTC) #27
Sebastian Noack
Looks mostly good to me, except for globals situation, which as discussed above, I expect ...
2 years, 4 months ago (2017-03-30 18:58:04 UTC) #28
kzar
Patch Set 24 : Tidy up hasRecord and some notification logic https://codereview.adblockplus.org/29374674/diff/29392699/chrome/ext/common.js File chrome/ext/common.js (right): ...
2 years, 4 months ago (2017-03-31 03:42:24 UTC) #29
kzar
Patch Set 25 : Remove two globals from tests
2 years, 4 months ago (2017-03-31 07:51:38 UTC) #30
Sebastian Noack
https://codereview.adblockplus.org/29374674/diff/29398605/lib/devtools.js File lib/devtools.js (right): https://codereview.adblockplus.org/29374674/diff/29398605/lib/devtools.js#newcode88 lib/devtools.js:88: record.request.type == "DOCUMENT" ? On 2017/03/31 03:42:18, kzar wrote: ...
2 years, 4 months ago (2017-03-31 07:59:45 UTC) #31
kzar
Patch Set 26 : Addressed more feedback https://codereview.adblockplus.org/29374674/diff/29398605/lib/devtools.js File lib/devtools.js (right): https://codereview.adblockplus.org/29374674/diff/29398605/lib/devtools.js#newcode88 lib/devtools.js:88: record.request.type == ...
2 years, 4 months ago (2017-03-31 08:27:44 UTC) #32
kzar
Patch Set 27 : Use .includes again
2 years, 4 months ago (2017-03-31 08:39:38 UTC) #33
Sebastian Noack
2 years, 4 months ago (2017-03-31 08:40:39 UTC) #34
LGTM!
Sign in to reply to this message.

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