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

(fixes #494) Ignore indent rules on files other than .vue #544

Merged
merged 1 commit into from
Aug 3, 2018

Conversation

michalsnik
Copy link
Member

As pointed in #494 indent rules affects all files, thus script-indent lints not only .vue files, but also .js files.

I think that we should encourage usage of native eslint indent rule and script-indent only for single file components, where people might want the extra base indentation.

@michalsnik michalsnik requested a review from chrisvfritz August 2, 2018 14:26
@chrisvfritz chrisvfritz requested a review from znck August 2, 2018 14:53
@mysticatea
Copy link
Member

LGTM, but this is a breaking change we should write in the changelog.

@michalsnik
Copy link
Member Author

We're still in beta phase @mysticatea and actually this change won't introduce more issues being reported, rather less - so it shouldn't break anything.

@michalsnik michalsnik merged commit 1c410d8 into master Aug 3, 2018
@michalsnik michalsnik deleted the enable-script-indent-only-on-vue branch August 3, 2018 08:38
@mysticatea
Copy link
Member

@michalsnik It's a surprising change to people which is using the rule for .js files as well. They will lose linting about indentation. It's the reason I said, "we should write in the changelog."

@chrisvfritz
Copy link
Contributor

@mysticatea I agree it should definitely be included in the changelog! 🙂 But is reducing the scope of a rule typically considered a "breaking change" in ESLint? In this context, I was thinking of a breaking change as anything that could produce potentially undesired errors. I'm quite inexperienced in the ESLint community though, so maybe I've been thinking about it wrong. 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants