Opened 3 years ago

Closed 3 years ago

Last modified 2 years ago

#4864 closed change (fixed)

Start using ESLint for adblockpluschrome repository — at Version 12

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: Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Ross Verified working: yes
Review URL(s):

https://codereview.adblockplus.org/29374674/

Description (last modified by kzar)

Background

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.

Change History (12)

comment:1 Changed 3 years ago by kzar

  • Blocked By 4871 added

comment:2 Changed 3 years ago by kzar

  • Description modified (diff)

comment:3 Changed 3 years ago by kzar

  • Blocked By 4871 removed
  • Description modified (diff)

comment:4 Changed 3 years ago by kzar

  • Blocked By 5060 added

comment:5 Changed 3 years ago by kzar

  • Blocked By 5060 removed

comment:6 Changed 3 years ago by kzar

  • Blocked By 5077 added

comment:7 Changed 3 years ago by kzar

  • Blocked By 5079 added

comment:8 Changed 3 years ago by abpbot

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

comment:9 Changed 3 years ago by kzar

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

comment:10 Changed 3 years ago by kzar

  • Blocked By 5079 removed

comment:11 Changed 3 years ago by kzar

  • Blocked By 5077 removed

comment:12 Changed 3 years ago 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.

Note: See TracTickets for help on using tickets.