Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

all: standardise tslint config #677

Merged
merged 1 commit into from
Oct 5, 2018
Merged

all: standardise tslint config #677

merged 1 commit into from
Oct 5, 2018

Conversation

43081j
Copy link
Contributor

@43081j 43081j commented Aug 29, 2018

Fixes #305.


Let me know if you still want this, otherwise i will throw it away.

  • Added missing lint commands
  • Made all tslint configs inherit the base one
  • Fixed a couple of lint errors
  • Added a lint script (npm run lint) to lint all packages

Couple of packages break the no-any rule so i turned it off as i didn't want to be altering interfaces for this.

Copy link
Contributor

@TimvdLippe TimvdLippe left a comment

Choose a reason for hiding this comment

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

What is the impact of the no-any rules? E.g. how many warnings are we getting? Would be great if we can remove that inconsistency as well, but probably better in a separate PR, potentially per package.

@43081j
Copy link
Contributor Author

43081j commented Aug 29, 2018

@TimvdLippe

no-any: false for:

  • cleankill
  • dom5
  • web-component-tester

the first two use any in parameters of functions so we need to check what uses those functions externally before changing it (e.g. a lot depends on dom5).

WCT just uses any all over the place...

Copy link
Contributor

@TimvdLippe TimvdLippe left a comment

Choose a reason for hiding this comment

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

This LGTM. Probably needs another look from someone on the tools team as well.

@usergenic
Copy link
Contributor

Thank you for this PR. Sorry for missing it earlier! Merging.

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

Successfully merging this pull request may close these issues.

None yet

4 participants