Opened 2 years ago

Closed 2 years ago

#5257 closed change (fixed)

`npm test` should run the linter

Reported by: hfiguiere Assignee: hfiguiere
Priority: P4 Milestone:
Module: Core Keywords:
Cc: kzar, trev, sebastian, fhd Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29441570

Description (last modified by hfiguiere)

Background

When the review says "eslint should have caught this" that means eslint didn't get run. The best way to ensure the developer does run eslint is it run eslint as part of the tests.

What to change

-Add eslint and the eyeo eslint config as a dependency in the package.json
-Run eslint as part of npm test
-Make sure the tests can run even if linter fails.

Bonus

It will also allow us to track the linter config version to the right one.

Change History (8)

comment:1 Changed 2 years ago by hfiguiere

  • Owner set to hfiguiere

comment:2 Changed 2 years ago by hfiguiere

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:3 Changed 2 years ago by hfiguiere

  • Description modified (diff)

comment:4 Changed 2 years ago by kzar

  • Cc kzar trev sebastian fhd added

comment:5 Changed 2 years ago by trev

  • Priority changed from Unknown to P4
  • Ready set

comment:6 Changed 2 years ago by abpbot

A commit referencing this issue has landed:
Issue 5257 - Add an eslint target to npm run and run it posttest.

comment:7 Changed 2 years ago by abpbot

A commit referencing this issue has landed:
Issue 5257 - Add an eslint target to npm run and run it posttest.

comment:8 Changed 2 years ago by hfiguiere

  • Resolution set to fixed
  • Status changed from reviewing to closed
Note: See TracTickets for help on using tickets.