Page MenuHomePhabricator

[cashtab] add arcanist linter support for eslint
Needs RevisionPublic

Authored by majcosta on Fri, Apr 30, 18:02.


Group Reviewers
Restricted Project

basic eslint recommended settings configuration and a helpful .arclint from

it does static analysis, catches js errors and is generally well regarded. we already have it as a devDependency for cashtab so we might as well run it and integrate with phab

Test Plan

arc diff this patch touching one file with a warning and a file with errors

fail to submit the patch if the errors aren't fixed see all linter warnings and errors in the diff body

Diff Detail

rABC Bitcoin ABC
Lint Passed
No Test Coverage
Build Status
Buildable 15610
Build 31136: Build Diff
Build 31135: arc lint + arc unit

Event Timeline

revert the linter complaints and green the diff

Fabien requested changes to this revision.Mon, May 3, 06:43
Fabien added a subscriber: Fabien.
Fabien added inline comments.

This is probably not doing what you expect. I suppose you meant (\\.jsx?$) ?


If it's empty you'd better remove it


What is the $0 here ?

There are a few reasons why I don't like script-and-regex, and here are some:

  • There is no version checking
  • There is no guarantee this will work on all platforms (does it run on macOS for example ?)
  • Performance suffers from calling a shell for each file to process. This has proven to be a problem in the past. In this case it's even worst: PHP is calling a shell, which calls a shell, which calls two shells (see your parenthesis), one of them ending up calling eslint.

I don't see a reason why this could not be a plain PHP linter that solves all of these issues. Let me know if you need help for writing it.

This revision now requires changes to proceed.Mon, May 3, 06:43