Navigation Menu

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

Conversation

mysticatea
Copy link
Member

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 This change enhances an existing feature of ESLint core Relates to ESLint's core APIs and features accepted There is consensus among the team that this change meets the criteria for inclusion breaking This change is backwards-incompatible do not merge This pull request should not be merged yet 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 This pull request should not be merged yet label Dec 23, 2019
# Conflicts:
#	lib/cli-engine/file-enumerator.js
#	tests/lib/_utils.js
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. There's a lot to this - nice work!

@btmills btmills merged commit 1aa021d into master Jan 17, 2020
v7.0.0 automation moved this from Implemented, pending review to Done Jan 17, 2020
@btmills btmills deleted the rfc20 branch January 17, 2020 16:32
@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 pull request 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 pull request 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 pull request 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 pull request 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 pull request 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 pull request 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 pull request 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 This issue has been archived; please open a new issue for any further discussion label Jul 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion breaking This change is backwards-incompatible core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint
Projects
No open projects
RFCs
  
Done
v7.0.0
  
Done
Development

Successfully merging this pull request may close these issues.

Support specifying extensions in the config
3 participants