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

Breaking: lint overrides files (fixes #10828, refs eslint/rfcs#20) #12677

Merged
merged 5 commits into from Jan 17, 2020
Merged

Conversation

mysticatea
Copy link
Member

@mysticatea mysticatea commented Dec 17, 2019

Don't merge until we finished releasing the last one of 6.x.

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

[X] Add something to the core

What changes did you make? (Give an overview)

This PR implements RFC20 then fixes #10828. If overrides[].files property matches, ESLint lints other files than JS even if --ext option is not present.

Main logics are:

  • ConfigArray#isAdditionalTargetPath(filePath) ... checks if a file path is matched with any overrides entry.
  • FileEnumerator#isTargetPath(filePath) ... checks if a file path is a lint target.

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

Nothing in particular.

@mysticatea mysticatea added enhancement core accepted breaking do not merge labels Dec 17, 2019
@mysticatea mysticatea added this to Implemented, pending review in v7.0.0 Dec 17, 2019
@mysticatea mysticatea added this to Implemented, pending review in RFCs Dec 17, 2019
@kaicataldo kaicataldo removed the do not merge label Dec 23, 2019
mysticatea added 2 commits Jan 7, 2020
# Conflicts:
#	lib/cli-engine/file-enumerator.js
#	tests/lib/_utils.js
Copy link
Member

@btmills btmills left a comment

LGTM. There's a lot to this - nice work!

@btmills btmills merged commit 1aa021d into master Jan 17, 2020
11 checks passed
v7.0.0 automation moved this from Implemented, pending review to Done Jan 17, 2020
@btmills btmills deleted the rfc20 branch Jan 17, 2020
@mysticatea mysticatea moved this from Implemented, pending review to Done in RFCs Jan 23, 2020
montmanu pushed a commit to montmanu/eslint that referenced this issue Mar 4, 2020
…s#20) (eslint#12677)

* Breaking: lint `overrides` files (fixes eslint#10828, refs eslint/rfcs#20)

* update docs

* sort for tests

See also: eslint#12700 (comment)
btmills added a commit that referenced this issue Apr 12, 2020
PR #12677, `Breaking: lint overrides files`, removed the default `.js`
value from the  default CLIEngine options. However, it did not remove
the same default value from the `--ext` option parsed by Optionator.
Because of this, when running `eslint .` without the `--ext` option,
Optionator would insert a default `--ext .js` value that would be passed
on to the CLIEngine. ESLint only lints `overrides` files if the `--ext`
CLI option is not passed, so the default `--ext .js` option prevented
ESLint from ever reaching the `overrides` file path checking flow.

but the `--ext` option's default
`.js` value in Optionator remained, so when running `eslint .` without
passing the `--ext` option,
btmills added a commit that referenced this issue Apr 12, 2020
PR #12677, `Breaking: lint overrides files`, removed the default `.js`
value from the  default CLIEngine options. However, it did not remove
the same default value from the `--ext` option parsed by Optionator.
Because of this, when running `eslint .` without the `--ext` option,
Optionator would insert a default `--ext .js` value that would be passed
on to the CLIEngine. ESLint only lints `overrides` files if the `--ext`
CLI option is not passed, so the default `--ext .js` option prevented
ESLint from ever reaching the `overrides` file path checking flow.
btmills added a commit that referenced this issue Apr 12, 2020
PR #12677, `Breaking: lint overrides files`, removed the default `.js`
value from the  default CLIEngine options. However, it did not remove
the same default value from the `--ext` option parsed by Optionator.
Because of this, when running `eslint .` without the `--ext` option,
Optionator would insert a default `--ext .js` value that would be passed
on to the CLIEngine. ESLint only lints `overrides` files if the `--ext`
CLI option is not passed, so the default `--ext .js` option prevented
ESLint from ever reaching the `overrides` file path checking flow.
btmills added a commit that referenced this issue Apr 14, 2020
PR #12677, `Breaking: lint overrides files`, removed the default `.js`
value from the  default CLIEngine options. However, it did not remove
the same default value from the `--ext` option parsed by Optionator.
Because of this, when running `eslint .` without the `--ext` option,
Optionator would insert a default `--ext .js` value that would be passed
on to the CLIEngine. ESLint only lints `overrides` files if the `--ext`
CLI option is not passed, so the default `--ext .js` option prevented
ESLint from ever reaching the `overrides` file path checking flow.
kaicataldo pushed a commit that referenced this issue Apr 23, 2020
PR #12677, `Breaking: lint overrides files`, removed the default `.js`
value from the  default CLIEngine options. However, it did not remove
the same default value from the `--ext` option parsed by Optionator.
Because of this, when running `eslint .` without the `--ext` option,
Optionator would insert a default `--ext .js` value that would be passed
on to the CLIEngine. ESLint only lints `overrides` files if the `--ext`
CLI option is not passed, so the default `--ext .js` option prevented
ESLint from ever reaching the `overrides` file path checking flow.
papandreou added a commit to unexpectedjs/unexpected that referenced this issue May 9, 2020
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Jul 17, 2020
@eslint-deprecated eslint-deprecated bot added the archived due to age label Jul 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted archived due to age breaking core enhancement
Projects
No open projects
RFCs
  
Done
v7.0.0
  
Done
Development

Successfully merging this pull request may close these issues.

3 participants