Page MenuHomePhabricator
Differential D9463

[cashtab] add arcanist linter support for eslint
AbandonedPublic
Actions

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

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Summary

basic eslint recommended settings configuration and a helpful .arclint from https://secure.phabricator.com/D12198

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

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

Event Timeline

majcosta created this revision.Apr 30 2021, 18:02
Herald added a reviewer: Restricted Project. · View Herald TranscriptApr 30 2021, 18:02
majcosta requested review of this revision.Apr 30 2021, 18:02
Harbormaster completed remote builds in B15609: Diff 28318.Apr 30 2021, 18:02
majcosta updated this revision to Diff 28319.Apr 30 2021, 18:05

revert the linter complaints and green the diff

Harbormaster completed remote builds in B15610: Diff 28319.Apr 30 2021, 18:05
majcosta edited the test plan for this revision. (Show Details)Apr 30 2021, 18:06
majcosta edited the summary of this revision. (Show Details)Apr 30 2021, 18:46
majcosta planned changes to this revision.Apr 30 2021, 19:48
majcosta requested review of this revision.Apr 30 2021, 22:28
Fabien requested changes to this revision.May 3 2021, 06:43
Fabien added a subscriber: Fabien.
Fabien added inline comments.
.arclint
297

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

298

If it's empty you'd better remove it

299

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.May 3 2021, 06:43
majcosta abandoned this revision.Dec 14 2022, 21:38

Revision Contents
Changeset List

PathSize
7 lines

Diff 28319

View Options

.arclint

Loading...