dojo / dojo-package-template

Dojo 2 - template to clone for creating packages (internal use)
Other
6 stars 18 forks source link

Start using linting rules from ESLint #96

Open umaar opened 7 years ago

umaar commented 7 years ago

Enhancement

There are lots of useful ESLint rules out there, and it'd be nice to explore how to use ESLint rules with TypeScript. I see there is this project: https://github.com/eslint/typescript-eslint-parser which could help. Here are some rules which I think are sensible: https://github.com/sindresorhus/eslint-config-xo/blob/891eb4c9bd1e33af8a12fd6f64d0629c08b88785/index.js#L16-L274

Context: https://github.com/dojo/routing/pull/110#discussion_r147370147

Also see: https://github.com/nzakas/eslint-plugin-typescript & https://github.com/buzinas/tslint-eslint-rules

kitsonk commented 7 years ago

It would be more than just looking at the rules. We would need to:

Since we have linting, to a degree already, and this is a non-trivial change, I think this effectively becomes a "nice to have" for the RC, scheduling as such.

umaar commented 7 years ago

Just to put a related resource into this ticket: there is a tslint-xo package which normally comes with a set of sensible defaults, and avoids the hassle of development teams having to configure rules one by one. It would be a:

{
    "extends": "tslint-xo"
}

In the tslint.json file for projects.

kitsonk commented 7 years ago

We have a fairly opinionated sense of linting already... While I have respect for other parts of the community, I would not want to tie something as personal and subjective to another project where they make decisions about our code and style guides. We could consider to adopt it, but I would rather iterate on our already established linting and style guide. We have 20 something packages and taking a churn to refactor that...

Also I would much rather us pursue dojo/meta#206. There are only a couple items that would contravene our current style guide and only one I really object to, which is an issue that is open and maybe resolved.

I guess I am struggling to understand why we want to establish standards for things we have already established standards for.

umaar commented 7 years ago

I only suggest it because I feel we have a large number of useful rules either turned off, or not included at all from the tslint.json. Also during code review, a number of things came up which were not caught automatically, things that I'd expect a linter to warn me about in my editor & on Travis (but even better is for prettier to fix it automatically as you say).