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: config extends dependency lookup (fixes #5023) #5070

Merged
merged 1 commit into from
Jan 26, 2016
Merged

Conversation

nzakas
Copy link
Member

@nzakas nzakas commented Jan 25, 2016

This was a bit trickier than I thought. I had to get rid of tests that were just too difficult to get working. However, the code coverage is basically the same, as the tests I removed were from cli and config, both of which have way more tests than they should (as all they do is hand off data to other utilities....a lot of tests are left over from when all the logic was in these two files).

@@ -371,6 +350,11 @@ function resolve(filePath) {
filePath = "eslint-config-" + filePath;
}

// some additional information processing is necessary
filePath = resolveModule.sync(filePath, {
basedir: relativeTo || process.cwd()
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to use process.cwd here? This will affect cwd CLIEngine flag.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm yeah. Maybe I can get away without a default here. Let me double-check.

Copy link
Member Author

Choose a reason for hiding this comment

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

Going to leave this for now. It will be an edge case for now, but I'll come back around and refactor later to do this correctly.

nzakas added a commit that referenced this pull request Jan 26, 2016
Update: config extends dependency lookup (fixes #5023)
@nzakas nzakas merged commit 689f56b into master Jan 26, 2016
@nzakas nzakas deleted the issue5023 branch January 26, 2016 21:51
@gyandeeps
Copy link
Member

After merging this PR, the build is broken.
I am trying to look into it also as it broken on my local machine also.

@gyandeeps
Copy link
Member

Found the reason, its broken because of the latest mocha release (3 hours ago).
Breaks on 2.4.1 but works on 2.3.4. they never released 2.4.0.

mochajs/mocha#2073

@nzakas
Copy link
Member Author

nzakas commented Jan 27, 2016

Weird. Thanks for finding this, I noticed just before I left.

@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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants