Add the tests to the eslint config and include every js file by default.

Yoast / yoast-components

Accessible React components by Yoast

GNU General Public License v3.0

21 stars 6 forks source link

Add the tests to the eslint config and include every js file by default. #741

Closed dutchmartin closed 5 years ago

dutchmartin commented 5 years ago

Summary

This PR can be summarized in the following changelog entry:

Relevant technical choices:

Including every .js file by default in ESlint.
Fixed various errors that occurred by deleting code after discussing it with @atimmer .

Test instructions

This PR can be tested by following these steps:

Run the unit tests by executing yarn test and eslint by executing grunt eslint

Fixes #737

IreneStr commented 5 years ago

CR: ❓ Some questions:

Why are app/ and redux/ excluded from the linting?
In composites/OnboardingWizard/tests/wizardTest.js, thehas correct initial state test has been removed. Is this intentional?
In the same file, there are some empty tests. If you're already removing non-functioning test, please remove those as well.

dutchmartin commented 5 years ago

@IreneStr

Those where generating a lot of errors, so i decided to not fix them since that would broaden up the scope of this issue to fix all code style issues in this repo. The folders are excluded because travis executes eslint and fails on errors, which would prevent this from being merged.
Yes, that is exactly what i meant by Fixed various errors that occurred by deleting code after discussing it with @atimmer. in the PR description.
This feels like scope creep to me, but I will do it after redefining the scope of this issue.

To sum it up: please define what should be done in this pr, and what has to become a separate issue.

dutchmartin commented 5 years ago

Please note that some things are fixed by eslint autofix. If there are any weird things that happend with that, the eslint config may have to be modified.