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

Enable eslint rules that got disabled #19408

Merged
merged 6 commits into from May 31, 2019

Conversation

Projects
None yet
2 participants
@rafeca
Copy link
Contributor

commented May 27, 2019

On #19302, a few eslint rules were disabled to keep that PR small and manageable, now it's the time to enable a few of them.

There are two rules that I haven't enabled:

  • standard/no-callback-literal: We have quite a few Atom public APIs that expect a callback with the result on the first argument, so to enable this rule we would have to either add a bunch of ignore comments or to create a quite big breaking change.
  • node/no-deprecated-api: This is a quite useful rule, but I haven't been able to enable it because it complains about using url.parse(). Changing this to the new URL() constructor would be a breaking change, since we're passing url objects to packages.

We can follow-up what to do for these two rules in separate PRs, to keep the discussion focused.

rafeca added some commits May 27, 2019

Enable no-use-before-define eslint rule
The changed codepath seems to be an unintentional mistake which was
added in 5c1a49f

I cannot see any reason to set a default argument to itself: this is
going to cause a ReferenceError when calling that function without that
argument and I guess if that was the intended behaviour the author would
have added an explicit check at the beginning of the function (or at
least a comment).

@rafeca rafeca requested review from as-cii and nathansobo May 27, 2019

@as-cii

as-cii approved these changes May 31, 2019

Copy link
Member

left a comment

Looks reasonable!

@rafeca rafeca merged commit 677bbb7 into master May 31, 2019

2 checks passed

Atom Pull Requests #20190527.2 succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@rafeca rafeca deleted the enable-no-useless-escape-eslint-rule branch May 31, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.