CorpGlory / types-grafana

MIT License
9 stars 22 forks source link

Added tslint (based on DefinitelyTyped) #18

Closed edgardmessias closed 4 years ago

edgardmessias commented 4 years ago

Added tslint based on DefinitelyTyped.

Next steps after this merge, are change tslint rules to more restrictive

edgardmessias commented 4 years ago

And the github workflow: https://github.com/edgardmessias/types-grafana/actions/runs/57607893

jonyrock commented 4 years ago

well, it's cool that code it better, but not sure here should be tslint-stylish.json, same thing with github/workflows/lint.yml

The whole problem with committing to open-source is that you need to review many things with very lack of time. So it would nice to have in PR only a small change which you can review in 5 mins at most.

edgardmessias commented 4 years ago

The tslint-stylish.json is a lot of rules to parse output of tslint, because there are some differences between tslint stylish and eslint stylish from actions/setup-node@v1.

The .github/workflows/lint.yml is a fast way to identify some problems in new pull request, after run, it can show in pull request some errors (search for github action annotations)

Like this, but for tslint: image

edgardmessias commented 4 years ago

At moment, the workflow is ignoring a lot of rules, look: https://github.com/CorpGlory/types-grafana/pull/18/files#diff-ace19bd0c04529e685320269e3c05de9R4 After this merge request, I can work to solve some rules, and normalize the source code (I will make pull requests, one rule at a time to keep small changes and facilitate the review)

jonyrock commented 4 years ago

@rozetko plz take a look at exact diff

rozetko commented 4 years ago

Looks good, don't like just 2 things:

  1. yarn usage (we use npm instead of yarn)
  2. don't understand why do we need build script
edgardmessias commented 4 years ago

@rozetko, I can change to use npm instead of yarn

About build script, it is necessary to force typescript validation, like references and syntax error. The DefinitelyTyped need a build script too, to pass in validation

rozetko commented 4 years ago

@edgardmessias thanks for contributing

edgardmessias commented 4 years ago

@rozetko, you are welcome

Now I can normalize the source code with tslint rules