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

fix: [#4204] Fix remaining eslint warnings - enable ESLint config #4241

Merged
merged 6 commits into from
Jul 19, 2022

Conversation

sw-joelmut
Copy link
Collaborator

Fixes #4204

Note: This PR lint script will successfully pass once all warnings are addressed. This is being tackled in separate PRs of the same issue.

Description

This PR updates the ESLint configuration adding new rules and disabling the ones regarding the any type, since it requires deeper analysis to not use the type. Moreover, we force the lint script to fail if there is at least one warning, preventing to introduce new ones in the future.

Note: we excluded the @typescript-eslint/no-explicit-any and @typescript-eslint/explicit-module-boundary-types to tackle those issues as a separate task, as they require deeper analysis and testing.

Specific Changes

.eslintrc.json

  • Added node: true to the env property, due to not recognizing multiple NodeJs types.
  • Disabled @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types and @typescript-eslint/ban-types, since there are many part of the source code that uses implicit or explicit any types, which requires deeper analysis to decide which type should use instead.
  • Added jsdoc/require-jsdoc rule to warn the user when JSDocs are not provided in public classes, methods, properties and functions (ignoring private and protected).

Testing

The following images show the configuration working when there are/aren't errors.
image

@sw-joelmut sw-joelmut requested a review from a team as a code owner May 12, 2022 16:42
@coveralls
Copy link

coveralls commented May 12, 2022

Pull Request Test Coverage Report for Build 2690427907

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.003%) to 84.483%

Totals Coverage Status
Change from base Build 2678878347: -0.003%
Covered Lines: 19900
Relevant Lines: 22308

💛 - Coveralls

@tracyboehrer
Copy link
Member

@sw-joelmut Can you take a look at this? Failing during yarn lint.

@ceciliaavila
Copy link
Collaborator

@sw-joelmut Can you take a look at this? Failing during yarn lint.

Hi @tracyboehrer, the lint check fails because there are new issues to fix. We'll fix them in a different PR and let you know.

@tracyboehrer tracyboehrer merged commit 57e2642 into main Jul 19, 2022
@tracyboehrer tracyboehrer deleted the southworks/update/eslint-config branch July 19, 2022 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix remaining eslint warnings
4 participants