Skip to content
This repository has been archived by the owner on Jan 2, 2025. It is now read-only.

Feat/add tsci lint #79

Merged
merged 6 commits into from
Jul 10, 2024
Merged

Feat/add tsci lint #79

merged 6 commits into from
Jul 10, 2024

Conversation

imrishabh18
Copy link
Member

Lint error shown

Screenshot 2024-07-10 at 3 52 26 PM

Closes issue - #13

Copy link

codesandbox bot commented Jul 10, 2024

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders

Open Preview

@imrishabh18 imrishabh18 requested a review from seveibar July 10, 2024 15:30
@seveibar
Copy link
Contributor

So I think tsci lint should not require the "target project" (meaning the project the person is running tsci dev in) to have eslint installed (we need to bundle it). I also think we should only run custom rules for tscircuit not normal linting rules. For example: If you run tsci lint it might say You have a capacitor without a unit value "100", try changing to "100uF"

@imrishabh18
Copy link
Member Author

@seveibar Take a look now, I think this is a good place to start. We can either add the plugins config in this object tscircuitPlugin, or later publish it as a whole plugin in a separate repo. This should be done, when we have some listed rules that needs to be implemented

Copy link
Contributor

@seveibar seveibar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok i do like where this is going. Can you remove eslint from the top level since we use biome (biome doesnt support plugins so eslint is better for target projects)

lib/cmd-fns/lint.ts Outdated Show resolved Hide resolved
lib/cmd-fns/lint.ts Outdated Show resolved Hide resolved
@imrishabh18 imrishabh18 requested a review from seveibar July 10, 2024 18:59
.eslintrc.js Outdated Show resolved Hide resolved
.eslintignore Outdated Show resolved Hide resolved
@@ -0,0 +1,112 @@
import * as ts from 'typescript';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the naming convention for this project is supposed to be ExportNameExact or kebab-case (usually kebab-case). Also the file name should always match the export, tscircuitLinter.ts should be lint-project.ts

FWIW I generally believe these rules shouldn't be enforced if there isn't a linter running on ci... so feel free to ignore suggestion to rename

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool! It's okay, I will rename. If the codebase is following a standard practice, then it's fine to follow it thoroughly.

@imrishabh18 imrishabh18 requested a review from seveibar July 10, 2024 19:59
@seveibar seveibar merged commit 9f7f2cc into main Jul 10, 2024
2 checks passed
@seveibar seveibar deleted the feat/add-tsci-lint branch July 10, 2024 21:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants