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

Use eslint-config-metarhia #227

Closed
wants to merge 1 commit into from
Closed

Use eslint-config-metarhia #227

wants to merge 1 commit into from

Conversation

aqrln
Copy link
Member

@aqrln aqrln commented Sep 7, 2017

This commit removes .eslintrc.yml in favor of using the
eslint-config-metarhia package. Three rules are turned off here,
because they trigger lots of linter errors:

  • arrow-parens — forces consistent parens usage in arrow functions
  • comma-dangle — forces VCS- and diff-friendly comma usage
  • handle-callback-err — ensures the errors are handled instead of
    being silently ignored

How these exceptions are handled further is outside the scope of this
commit/pull request, and they might need somewhat deeper investigation
than just considering the rules themselves. E.g., errors triggered by
handle-callback-err may not necessarily mean that there's a lot of
places errors are not handled in metasync (though if that's indeed the
case, it will show nicely where to fix these bugs so that one doesn't
need to spot all of them manually), but it might be just that there's
some trouble with regex specified in this rule's settings.

One of the linter errors is handled in this commit though: there was a
require() call pointing to a non-existent file, which is now fixed.

Also, eslint package has been updated to match what peerDependencies
of eslint-config-metarhia require, and eslint-plugin-import, which
is also a part of the config's peerDependencies, has been installed.

This commit removes `.eslintrc.yml` in favor of using the
`eslint-config-metarhia` package.  Three rules are turned off here,
because they trigger lots of linter errors:

 - `arrow-parens` — forces consistent parens usage in arrow functions
 - `comma-dangle` — forces VCS- and diff-friendly comma usage
 - `handle-callback-err` — ensures the errors are handled instead of
   being silently ignored

How these exceptions are handled further is outside the scope of this
commit/pull request, and they might need somewhat deeper investigation
than just considering the rules themselves.  E.g., errors triggered by
`handle-callback-err` may not necessarily mean that there's a lot of
places errors are not handled in metasync (though if that's indeed the
case, it will show nicely where to fix these bugs so that one doesn't
need to spot all of them manually), but it might be just that there's
some trouble with regex specified in this rule's settings.

One of the linter errors is handled in this commit though: there was a
`require()` call pointing to a non-existent file, which is now fixed.

Also, `eslint` package has been updated to match what `peerDependencies`
of `eslint-config-metarhia` require, and `eslint-plugin-import`, which
is also a part of the config's `peerDependencies`, has been installed.
@aqrln
Copy link
Member Author

aqrln commented Sep 7, 2017

I've taken another look, and yeah, handle-callback-err failures are legitimate, but all of them are in tests, not in the library code itself. I'd say land this as-is, and then remove "handle-callback-err": "off" from .eslintrc.json and add test.notOk(error, 'operation must not return an error'); where necessary as a separate commit.

@belochub
Copy link
Member

Closed in favor of #379.

@belochub belochub closed this Oct 31, 2018
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.

2 participants