Page MenuHomePhabricator

[arclint] Fix arc lint for Cashtab
ClosedPublic

Authored by bytesofman on Nov 20 2024, 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 31221
Build 61942: Build Diffcashtab-tests
Build 61941: arc lint + arc unit

Event Timeline

Fabien added inline comments.
cashtab/.eslintrc.js
40

Why not add the exclusion to the .arclint file as well, so it's all in a single place ?

bytesofman added inline comments.
cashtab/.eslintrc.js
40

The general pattern is that arclint includes 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.

This revision is now accepted and ready to land.Nov 20 2024, 14:40
This revision was automatically updated to reflect the committed changes.
bytesofman marked an inline comment as done.