Opened on 02/06/2017 at 06:08:19 AM

Closed on 03/31/2017 at 09:13:50 AM

Last modified on 07/06/2017 at 01:14:05 PM

#4864 closed change (fixed)

Start using ESLint for adblockpluschrome repository

Reported by: kzar Assignee: kzar
Priority: P2 Milestone: Adblock-Plus-1.13.3-for-Chrome-Opera
Module: Platform Keywords:
Cc: sebastian, Ross, rraceanu, philll, sclow Blocked By: #3692
Blocking: #5080 Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Ross Verified working: yes
Review URL(s):

Description (last modified by kzar)


We've created an ESLint configuration for the company eslint-config-eyeo (see #3692). Now we need to make changes to adblockpluschrome to use ESLint and that configuration.

What to change

  • Add a ESLint configuration to adblockpluschrome which inherits from eslint-config-eyeo:
  "extends": "eslint-config-eyeo",
  "root": true,
  • Add an ESLint ignore file to ignore files such as jQuery.
  • Make the necessary changes to the adblockpluschrome code so that linting passes.
  • Add a note to the README explaining how to lint the code. (eslint *.js lib/ qunit/ ext/ chrome/)

Hints for testers

Unfortunately these changes were pretty comprehensive and will require very thorough testing of pretty much all code paths. Please take some time to test this on Chrome 49, a recent version such as 57 and Opera. I recommend testing this along side #5061 (and perhaps #5023) since that also contained large sweeping changes. Really anything and everything you can think of needs to be tested here.

Attachments (0)

Change History (16)

comment:1 Changed on 02/08/2017 at 06:36:03 AM by kzar

  • Blocked By 4871 added

comment:2 Changed on 02/08/2017 at 06:39:21 AM by kzar

  • Description modified (diff)

comment:3 Changed on 02/08/2017 at 09:04:51 AM by kzar

  • Blocked By 4871 removed
  • Description modified (diff)

comment:4 Changed on 03/30/2017 at 06:36:25 AM by kzar

  • Blocked By 5060 added

comment:5 Changed on 03/31/2017 at 05:52:03 AM by kzar

  • Blocked By 5060 removed

comment:6 Changed on 03/31/2017 at 05:59:53 AM by kzar

  • Blocked By 5077 added

comment:7 Changed on 03/31/2017 at 07:34:30 AM by kzar

  • Blocked By 5079 added

comment:8 Changed on 03/31/2017 at 08:46:29 AM by abpbot

A commit referencing this issue has landed:
Issue 4864 - Start using ESLint for adblockpluschrome

comment:9 Changed on 03/31/2017 at 08:47:52 AM by kzar

  • Milestone set to Adblock-Plus-for-Chrome-Opera-next

comment:10 Changed on 03/31/2017 at 09:01:01 AM by kzar

  • Blocked By 5079 removed

comment:11 Changed on 03/31/2017 at 09:01:50 AM by kzar

  • Blocked By 5077 removed

comment:12 Changed on 03/31/2017 at 09:13:50 AM by kzar

  • Cc Ross rraceanu philll sclow added
  • Description modified (diff)
  • Resolution set to fixed
  • Review URL(s) modified (diff)
  • Status changed from new to closed

I've opened #5080 for the changes required to reduce the /* globals ... */ comments. Originally I was going to reuse this one but I realised that was probably a mistake.

Phill / Ross / Robert / Scott - Heads up testing this, #5061 and #5023 will require a whole bunch of work. I recommend combining testing of those three and starting soon as possible. There is a high likelihood of regressions with all these changes, we don't want to miss them or find them just before the next release.

comment:13 Changed on 03/31/2017 at 09:20:17 AM by sebastian

As for testing, note that the devbuilds on the Chrome Web Store don't get updated at the moment due to an issue on their end we are waiting for to get fixed, see ticket:5027#comment:25.

comment:14 Changed on 03/31/2017 at 11:49:02 AM by kzar

  • Blocking 5080 added

comment:15 Changed on 04/03/2017 at 08:00:37 AM by Ross

kzar / sebastian: Have picked this up.

comment:16 Changed on 07/06/2017 at 01:14:05 PM by Ross

  • Tester changed from Unknown to Ross
  • Verified working set

Done. The build appears to working as expected across a variety of browser versions.

Chrome 49 / 54 / 58 / Windows 7
Opera 36 / 42 / 45 / Windows 7

Add Comment

Modify Ticket

Change Properties
as closed .
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from kzar.
Note: See TracTickets for help on using tickets.