[arclint] Fix arc lint for Cashtab
ClosedPublic
Actions

Authored by bytesofman on Wed, Nov 20, 14:24.

Details

Reviewers

Fabien

Group Reviewers

Restricted Project

Commits

rABC574f7d473f55: [arclint] Fix arc lint for Cashtab

Summary

There are some complications in linting Cashtab, which now includes ts files and js files. Some js files are pre-es6 js (build and config scripts related to create react app). These should all eventually be upgraded as well. For now, though, they are boilerplate.

Stop linting these files as ts linter does not handle pre-es6 js.

Adjust arc lint to include ts files for cashtab.

Test Plan

arc lint -- cashtab/**/*

Diff Detail

Repository

rABC Bitcoin ABC

Branch

cashtab-fix-linter

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 31220
Build 61940: Build Diff	cashtab-tests
Build 61939: arc lint + arc unit

Event Timeline

bytesofman created this revision.Wed, Nov 20, 14:24

Herald added a reviewer: Restricted Project. · View Herald TranscriptWed, Nov 20, 14:24

improve comments

Harbormaster completed remote builds in B31220: Diff 50977.Wed, Nov 20, 14:28

Harbormaster completed remote builds in B31221: Diff 50978.

bytesofman published this revision for review.Wed, Nov 20, 14:30

Fabien added a subscriber: Fabien.Wed, Nov 20, 14:33

Fabien added inline comments.

cashtab/.eslintrc.js
40 ↗	(On Diff #50978)	Why not add the exclusion to the .arclint file as well, so it's all in a single place ?

bytesofman marked an inline comment as done.Wed, Nov 20, 14:39

bytesofman added inline comments.

cashtab/.eslintrc.js
40 ↗	(On Diff #50978)	The general pattern is that arclint `include`s the top level directory. app-specific exclusions are handled by the respective `.eslintignore` files in each dir. What we choose to ignore is often dynamic and imo best handled at the level of the specific app / subdir in the monorepo. This is a weird case in Cashtab where we have a JS app, initially a Create React App, which uses nodejs scripts to build the react app -- now ejected from Create React App (to customize those scripts for polyfills related to crypto), and (partially) converted to typescript. I'm still figuring out some of the ts settings and how they interact with things like jest and eslint. In this case, the ts linter is using tsconfig.json to decide what to lint. because tsconfig.json is not including the build dir (sensibly), build is not being picked up by the eslintignore. This kind of weird edge case is the sort of thing that is best handled at the cashtab/ level and not arc lint. Going forward, will probably find a way to modify tsconfig.json so that eslintignore works. However, this tsconfig.json is still being updated/changed as I convert more of Cashtab to ts. So for now, excluding these dirs is probably the best way to fix the linter and get the benefit of CI linting.