vscode-markdown-notes icon indicating copy to clipboard operation
vscode-markdown-notes copied to clipboard

Eslint throws errors. Add lint to CI?

Open NekoApocalypse opened this issue 3 years ago • 2 comments

Describe the bug

Running npm run lint after pulling the master branch throws the following errors:

> [email protected] lint
> eslint -c .eslintrc.js --ext .ts src

.../vscode-markdown-notes/src/NoteParser.ts
  164:1  error  Expected indentation of 8 spaces but found 10  @typescript-eslint/indent

/mnt/5034559134557B44/work_2021/vscode-markdown-notes/src/NoteWorkspace.ts
  218:1  error  Expected indentation of 4 spaces but found 6  @typescript-eslint/indent
  320:1  error  Expected indentation of 4 spaces but found 6  @typescript-eslint/indent
  321:1  error  Expected indentation of 4 spaces but found 6  @typescript-eslint/indent
  322:1  error  Expected indentation of 4 spaces but found 6  @typescript-eslint/indent
  323:1  error  Expected indentation of 4 spaces but found 6  @typescript-eslint/indent
  326:1  error  Expected indentation of 4 spaces but found 6  @typescript-eslint/indent
  327:1  error  Expected indentation of 6 spaces but found 8  @typescript-eslint/indent
  329:1  error  Expected indentation of 4 spaces but found 6  @typescript-eslint/indent

.../vscode-markdown-notes/src/test/jest/extension.test.ts
   30:6   error  Missing semicolon                             @typescript-eslint/semi
   31:4   error  Missing semicolon                             @typescript-eslint/semi
  216:82  error  Missing semicolon                             @typescript-eslint/semi
  219:74  error  Missing semicolon                             @typescript-eslint/semi
  222:76  error  Missing semicolon                             @typescript-eslint/semi
  229:51  error  Missing semicolon                             @typescript-eslint/semi
  235:7   error  Missing semicolon                             @typescript-eslint/semi
  236:5   error  Missing semicolon                             @typescript-eslint/semi
  286:1   error  Expected indentation of 6 spaces but found 8  @typescript-eslint/indent
  287:1   error  Expected indentation of 6 spaces but found 8  @typescript-eslint/indent
  288:1   error  Expected indentation of 6 spaces but found 8  @typescript-eslint/indent
  289:1   error  Expected indentation of 6 spaces but found 8  @typescript-eslint/indent
  293:1   error  Expected indentation of 6 spaces but found 8  @typescript-eslint/indent

✖ 22 problems (22 errors, 0 warnings)
  22 errors and 0 warnings potentially fixable with the `--fix` option.

To Reproduce

Steps to reproduce the behavior:

  1. Pull source code
  2. Run npm install
  3. Run npm run lint

Expected behavior

The master branch should not contain linter errors.

I guess this should be regarded as a bug..? I suggest add a lint steop to CI to prevent further lint issues.

NekoApocalypse avatar Apr 22 '21 06:04 NekoApocalypse

YES, I LOVE this idea. Any interest in opening a PR that does this? If not, I can figure it out eventually, but would be super helpful if someone who knows more about this could take a stab (and do a PR that addresses these issues).

Also, we should make sure to have whatever is the defacto eslint config in the repo

kortina avatar Apr 22 '21 15:04 kortina

From my experience Prettier + ESLint works quite well out of the box. Airbnb config is also quite popular, I guess it's a good idea to have it. The auto-formatting from Prettier is a life changer.

Also, I think it is better to include linter in the compile command, instead of adding it as a separate CI step.

NekoApocalypse avatar Apr 24 '21 10:04 NekoApocalypse