-
Notifications
You must be signed in to change notification settings - Fork 454
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
feat(config): add support for multiple paths in module name mapper #1690
Conversation
Pull Request Test Coverage Report for Build 4990
💛 - Coveralls |
hi, thank you for your PR. In general the PR LGTM. Just extra requests:
|
I have no idea how an e2e test should look like. Also I don't think there is any need for updating docs because it never explained the method's implementation details. |
You can put the example of using this function in external-repos. In that folder, there are several independent projects. You can adjust one project to have this function in use. That would be sufficient enough. To test the changes in external-repos, you can run test:external-repos script which can be found in package.json Regarding to documentation, it can be excluded from this PR. |
I added e2e tests, thanks for guiding me. Also rebased the master. I hope it passes the quality control this time :] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks 👍 LGTM
@kulshekhar please have a look once
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me as well. Thanks @OrkhanAlikhanov 😃
Summary
Closes #1072
Does this PR introduce a breaking change?