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

Allow additional ESLint config file names. #173

Merged
merged 3 commits into from
Nov 15, 2016
Merged

Allow additional ESLint config file names. #173

merged 3 commits into from
Nov 15, 2016

Conversation

goloroden
Copy link

According to ESLint documentation, a config file may have an extension. Hence I replaced the hard-coded comparison by a regular expression that matches all possible file names.

According to [ESLint documentation](http://eslint.org/docs/user-guide/configuring#configuration-file-formats), a config file may have an extension. Hence I replaced the hard-coded comparison by a regular expression that matches all possible file names.
@goloroden
Copy link
Author

I have seen that two of the three build steps failed, but IMHO this is fine, because:

  • codecov/project is yellow for the other PRs, too. Apparently there is something not working.
  • TravisCI successfully ran the tests, but failed when trying to talk to a coverage url. So the part that is really important passed.
  • AppVeyor is green :-)

@lijunle
Copy link
Member

lijunle commented Nov 15, 2016

@goloroden Thanks very much for contributing this project. The code looks good to me. Just one more point, could you please modify test code to cover the support? I suppose to add a for-loop in around this line.

BTW, I will handle this test-coverage is failing. Don't worry about that. 😺

@goloroden
Copy link
Author

Thanks for your fast response :-)

I have added to the tests what was needed to test the possible options.
Looking forward to the merge :-)

@lijunle lijunle merged commit a9682f1 into depcheck:master Nov 15, 2016
@lijunle
Copy link
Member

lijunle commented Nov 15, 2016

Merged! Thank you for helping this!

@lijunle
Copy link
Member

lijunle commented Nov 15, 2016

@goloroden Do you want a new version on this?

@goloroden
Copy link
Author

Basically I'd say yes, but maybe let's wait until the next PR regarding issue #174 is also merged (I need both of them for a project I'm currently working on). Is that okay for you?

@lijunle
Copy link
Member

lijunle commented Nov 15, 2016

Sure! 👍

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

Successfully merging this pull request may close these issues.

None yet

2 participants