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

Fix: Correct error and test messages to fit config search path #9428

Merged
merged 1 commit into from Oct 14, 2017
Merged

Fix: Correct error and test messages to fit config search path #9428

merged 1 commit into from Oct 14, 2017

Conversation

jrpool
Copy link
Contributor

@jrpool jrpool commented Oct 13, 2017

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

[ ] Documentation update
[X] 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)
Corrected error message and test description so they no longer exclude home-directory config files from set of directories searched for config files.

Is there anything you'd like reviewers to focus on?
The change to the test description is intended to be temporary, pending a change of the test to make it ignore config files in the user’s home directory. If reviewers have any reservations about the idea of making that change to the test and, along with it, restoring the original description, I would appreciate their comments on that.

@eslintbot
Copy link

LGTM

@kaicataldo kaicataldo added bug ESLint is working incorrectly core Relates to ESLint's core APIs and features evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion labels Oct 13, 2017
@kaicataldo
Copy link
Member

Thanks for the PR! Do you mind explaining what the problem is that you're trying to solve with this PR?

@not-an-aardvark
Copy link
Member

@kaicataldo I think this is related to the discussion in #9426.

@jrpool
Copy link
Contributor Author

jrpool commented Oct 13, 2017

This PR arises from comments on my merged PR #9426 by @platinumazure and @not-an-aardvark. The search for config files ends with a search in the user’s home directory if no others are found, but the error message and test description make it seem as if that does not happen. This can lead to confusion about why the test is failing. This PR aims to prevent that confusion by more fully describing the search.

@kaicataldo
Copy link
Member

Understood - thanks!

@kaicataldo
Copy link
Member

If reviewers have any reservations about the idea of making that change to the test and, along with it, restoring the original description, I would appreciate their comments on that.

I think this is fine as an intermediary step, since it accurately describes what is going on.

Copy link
Member

@ilyavolodin ilyavolodin left a comment

Choose a reason for hiding this comment

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

LGTM

@not-an-aardvark not-an-aardvark added chore This change is not user-facing and removed bug ESLint is working incorrectly core Relates to ESLint's core APIs and features evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion labels Oct 14, 2017
@ilyavolodin ilyavolodin merged commit 85388fb into eslint:master Oct 14, 2017
@kaicataldo
Copy link
Member

Thanks for contributing to ESLint!

@jrpool jrpool deleted the homeconfig branch October 14, 2017 21:09
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Apr 13, 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 Apr 13, 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 chore This change is not user-facing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants