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

remove no-throw-literal lint exception not needed #81

Merged
merged 1 commit into from
Jul 18, 2019

Conversation

brodycj
Copy link

@brodycj brodycj commented Jul 14, 2019

UPDATED:

As there is desire to keep the custom no-unused-vars entry, there seems to be only one exception that we can remove without updating the code. This is why I updated the title and force-pushed the change with the updated commit message.

Copy link
Member

@janpio janpio left a comment

Choose a reason for hiding this comment

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

This file is usually copied over to all the projects to enforce a standard that is valid across all our projects. Removing individual rules here that don't apply to this project will break that.

But if this really is a change in how we want to write our code in general, and there is a good reason behind it, this is of course fine - but it should be explained then.

@raphinesse
Copy link
Contributor

@janpio Actually, this file isn't consistent across all projects AFAIK. As a matter of fact, I added the no-unused-vars rule here (and only here) myself.

However, I agree that it would be great to enforce a consistent code style across all projects. But we should not do that by copying .eslintrc files but rather by having a Cordova ESLint config package that is required by all repos. I've had that on my mental to do list for quite some time but didn't get around to creating an issue for it.

Copy link
Contributor

@raphinesse raphinesse left a comment

Choose a reason for hiding this comment

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

no-unused-vars is a very useful addition IMHO. In fact I added it myself IIRC.

OTOH, removing the disabling of no-throw-literal would be more than fine with me.

@raphinesse
Copy link
Contributor

@janpio See apache/cordova#142

@brodycj brodycj changed the title remove (semi)standard eslint exceptions not needed remove no-throw-literal lint exception not needed Jul 18, 2019
@brodycj brodycj force-pushed the remove-lint-exceptions-not-needed branch from 8e35cc0 to f739ebe Compare July 18, 2019 16:10
@brodycj brodycj requested review from janpio, raphinesse and erisu and removed request for dpogue and erisu July 18, 2019 16:18
@brodycj
Copy link
Author

brodycj commented Jul 18, 2019

I just updated the title and force-pushed the change to remove the single no-throw-literal exception. Thanks @raphinesse for reviewing and raising apache/cordova#142 in response.

A quick review would be really appreciated.

P.S. I see that @raphinesse added no-unused-vars with the args: after-used setting in 8d6511e. I would love to see this in Standard JS itself, will raise a proposal for upcoming version 14.

@raphinesse raphinesse merged commit cbe9632 into apache:master Jul 18, 2019
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.

3 participants