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

Update: add --fix to no-debugger (fixes #8659) #8660

Merged
merged 1 commit into from Jun 2, 2017

Conversation

aladdin-add
Copy link
Member

@aladdin-add aladdin-add commented May 28, 2017

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

[x] Add autofixing to a rule

What changes did you make? (Give an overview)
add --fix to no-debugger. (fixes #8659 )

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

@eslintbot
Copy link

LGTM

@mention-bot
Copy link

@aladdin-add, thanks for your PR! By analyzing the history of the files in this pull request, we identified @nzakas, @vitorbal and @btmills to be potential reviewers.

@aladdin-add aladdin-add changed the title Update: add --fix to no-debugger Update: add --fix to no-debugger (fixes #8659) May 28, 2017
@aladdin-add
Copy link
Member Author

should also update the docs?

@soda0289
Copy link
Member

@aladdin-add Please update the docs. Checkout this commit for a sample of what should be added.
https://github.com/eslint/eslint/pull/7186/files

@soda0289 soda0289 added enhancement This change enhances an existing feature of ESLint evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion rule Relates to ESLint's core rules labels May 28, 2017
@eslintbot
Copy link

LGTM

@ilyavolodin
Copy link
Member

Updating docs is no longer required. Build will automatically insert appropriate text in the right spot for rules that are fixable.

Copy link
Member

@btmills btmills left a comment

Choose a reason for hiding this comment

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

Sorry for the confusion around docs. Can you remove the change to the docs? Then this will be ready to go!

@eslintbot
Copy link

LGTM

Copy link
Member

@btmills btmills 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 @aladdin-add!

@aladdin-add
Copy link
Member Author

aladdin-add commented Jun 2, 2017

@btmills done.

I thought the build will check it...

@ilyavolodin ilyavolodin merged commit 0058b0f into eslint:master Jun 2, 2017
@aladdin-add aladdin-add deleted the rule/no-debugger branch June 2, 2017 21:01
@kachkaev
Copy link

kachkaev commented Jul 17, 2017

Not sure the change complies with

  1. Avoid any fixes that could change the runtime behavior of code and cause it to stop working.

I wanted to use debugger; in dev and expected it to not automatically disappear on save due to prettier/eslint autofix. At the same time, I wanted debugger; to cause an error in npm run lint so that any case where this line is accidentally committed failed in CI. How to achieve such a behaviour after this PR?

Temporary have to write

// eslint-disable-next-line
debugger;

+300% to length in each case 😃

Not sure autofixing no-debugger is a good idea.

@platinumazure
Copy link
Member

@kachkaev Long-term, what we are hoping is that editor integrations will be able to go through fixable errors and choose which autofixes to apply or not. Recent versions of ESLint should provide all the information the integration would need (i.e., which rule is reporting a particular violation, whether or not the error is fixable, etc.); it should be possible for the integration to filter out certain types of fixable errors and just report them without fixing them.

So in short, I would recommend filing an issue on the GitHub repo for your editor integration.

@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Feb 6, 2018
@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 Feb 6, 2018
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 enhancement This change enhances an existing feature of ESLint evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion rule Relates to ESLint's core rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

proposal: add --fix to no-debugger
8 participants