Skip to content
This repository has been archived by the owner on Dec 10, 2020. It is now read-only.

Upgrade to @ethereumjs/eslint-config and lint #153

Merged
merged 17 commits into from
Oct 23, 2020
Merged

Conversation

cgewecke
Copy link
Contributor

@cgewecke cgewecke commented Oct 21, 2020

#148

NB: PR targets #152

Lints everything. Mostly automated fixes but have put the things done manually in a sequence of separate commits.

The meaningful config commits are:

  • 002e886: package upgrade and initial config
  • db45450: addition of files not enumerated by tsconfig
  • 074994e: turn "no-unused-vars" rule off

The pre-commit hook works out of the box, it's great.

// Duplicates the (more tolerant) @typescript-eslint/no-unused-vars
'no-unused-vars': 'off'
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Small note about parserOptions... was getting an error complaining of files unreachable through the tsconfig similar to:

error: Parsing error: "parserOptions.project" has been set for @typescript-eslint/parser.
The file does not match your project config: unusedFile.ts.
The file must be included in at least one of the projects provided at unusedFile.ts:

Have followed the suggestions at typescript-eslint#parserOptions.project to add the browser folder files and webpack.config.js to the lint set.

@@ -99,7 +98,8 @@ export class Libp2pServer extends Server {
})
})
}
(this.node as Libp2pNode).on('peer:discovery', async (peerInfo: any) => {
// eslint-disable-next-line no-extra-semi
;(this.node as Libp2pNode).on('peer:discovery', async (peerInfo: any) => {
Copy link
Contributor Author

@cgewecke cgewecke Oct 21, 2020

Choose a reason for hiding this comment

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

There were 8 locations where prettier requires we insert a semicolon to prevent potential ASI failures. However eslint (correctly?) complains these are unnecessary - e.g. their views conflict.

Deferred to prettier in this case because otherwise its semi rule would need to be turned off. (Not sure this is the right thing though...)

Base automatically changed from upgrade/config-typescript to master October 23, 2020 08:39
@coveralls
Copy link

coveralls commented Oct 23, 2020

Coverage Status

Coverage increased (+1.6%) to 76.195% when pulling 95acba1 on upgrade/eslint-config into b4d885b on master.

Copy link
Member

@holgerd77 holgerd77 left a comment

Choose a reason for hiding this comment

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

Whew, great, this could really be much quicker realized than I expected after my experiences with the monorepo battling. Cool! 😀

@holgerd77 holgerd77 merged commit f774a4b into master Oct 23, 2020
@holgerd77 holgerd77 deleted the upgrade/eslint-config branch October 23, 2020 09:10
@cgewecke
Copy link
Contributor Author

@holgerd77

I don't understand this sentence. Isn't the deactivated rule this mentioned rule from above?

Ah, actually every no-unused-var was being reported twice, once by the "regular" eslint rule, and once by @typescript-eslint/no-unused-var.

The latter is more permissive - if you rename the var with an _ prefix it goes quiet.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants