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

Docs: no-console#When-Not-To-Use provides incorrect rule snippet #11093

Conversation

choznerol
Copy link
Contributor

@choznerol choznerol commented Nov 17, 2018

What is the purpose of this pull request? (put an "X" next to item)

  • Documentation update
  • Bug fix (template)
  • New rule (template)
  • Changes an existing rule (template)
  • Add autofixing to a rule
  • Add a CLI option
  • Add something to the core
  • Other, please explain:

What changes did you make? (Give an overview)

The snippet provided in no-console#When Not To Use It aims to achieve "only receiving errors for console calls" but the selector results in a warning for console.log() instead of console.foobar().
screen shot 2018-11-17 at 12 17 05 pm

To fix it, the 'unexpected property' should be selected with != instead of =. Also, turning off "no-console" is still required to achieve "only receiving errors for console calls with unexpected property"

After change:
console.log() is ok
console.logs() is warned
default

Is there anything you'd like reviewers to focus on?

Current selector results in a warning for `console.log()` instead of `console.foobar()`. The 'unexpectd property' should be  selected with `!=`

Also, turning off "no-console" is still required to achieve "only receiving errors for console calls with unexpected property"

Add missing "no-console: off" for `When Not To Use It` snippet
@eslint-deprecated eslint-deprecated bot added the triage An ESLint team member will look at this issue soon label Nov 17, 2018
Copy link
Member

@platinumazure platinumazure left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

I believe the Travis failure is due to an unrelated issue.

@platinumazure platinumazure added rule Relates to ESLint's core rules documentation Relates to ESLint's documentation and removed triage An ESLint team member will look at this issue soon labels Nov 17, 2018
@choznerol
Copy link
Contributor Author

cool! but do i need to do anything like trigger a rebuild?

@platinumazure
Copy link
Member

@choznerol Nope, we'll handle it 😄

Copy link
Member

@aladdin-add aladdin-add left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@platinumazure platinumazure merged commit d2d500c into eslint:master Nov 20, 2018
@platinumazure
Copy link
Member

@choznerol Merged, thanks for contributing! (And apologies for the delay in getting this in.)

@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators May 20, 2019
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label May 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion documentation Relates to ESLint's documentation rule Relates to ESLint's core rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants