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

JS Linting #41

Merged
merged 6 commits into from
Jan 26, 2021
Merged

JS Linting #41

merged 6 commits into from
Jan 26, 2021

Conversation

tfrommen
Copy link
Contributor

@tfrommen tfrommen commented Jan 7, 2021

This PR targets JavaScript-specific linting improvements or changes to be made that have been mentioned in #38.

In more detail, these include:

  • Update relevant ESLint and TypeScript dependencies.
  • ESLint:
    • Update ruleset for TypeScript.
    • Lint both JavaScript and TypeScript files.
  • Run ESLint and stylelint on CI.
  • TypeScript:
    • Improve compiler options.
    • Do not lint "lib" folder.

This PR conflicts with #42 in that both make changes to the package.json file. If you want to merge both PRs, you should be able to manually accept all changes—make sure to keep the most recent package version. If you do not or only partially merge this PR, you will have to adapt some JavaScritp/TypeScript code! This PR currently fails on CI because it does not fix any linting errors in the production code. This is done in #42.

This was referenced Jan 7, 2021
"jsdoc/require-returns": "off",
"no-unused-vars": "off",
"no-use-before-define": "off"
},
Copy link
Member

Choose a reason for hiding this comment

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

You might be interested to know that I was making at attempt to not put any rule overrides in place, mostly to see whether it was practical or possible. I'm always concerned when I open a project and there are a hundred rule overrides and they only affect coding style. These all look like they make sense though.

Copy link
Contributor Author

@tfrommen tfrommen Jan 26, 2021

Choose a reason for hiding this comment

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

These are all sort-of required/recommended, since you are using TypeScript. There is even official documentation around this, but I would have to search for it.
But yeah, I agree that lots of overrides/disable statements is not a good sign.

@johnbillion johnbillion merged commit 7e6613d into develop Jan 26, 2021
@johnbillion johnbillion deleted the js-linting branch January 26, 2021 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants