[FEATURE] Add module_alias resolution capabilities - round 2
#8098
Conversation
add tests for resolve_alias
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed. If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
| ~reader | ||
| loc | ||
| ?resolution_acc | ||
| dir (spf "%s%s%s" dirname Filename.dir_sep r))) ) |
There was a problem hiding this comment.
I can implement the same modified_dir logic here if it's also preferred to support <PROJECT_ROOT> for module.system.node.resolve_dirname
davidroeca
force-pushed
the
feature/resolve_aliases
branch
from
0171bd9 to
1069672
Compare
|
@nmote I added some of the incremental tests that were mentioned in the previous PR and think this is ready for review as long as we're ok with only supporting |
|
@nmote anything? |
|
Sorry for the delay. I think I'll be able to take a close look at this sometime this week. |
|
I've looked a little bit closer and @gabelevi's recent work does seem to be pretty similar to this. Could you let me know whether what he's done works for your goals? If not, could you explain what you need from this PR that his work doesn't accomplish, and I'll take another look? Thanks for your patience on this. |
|
Looks to me like that PR should handle the main use cases I had in mind |
|
If that's the case, should this be closed? |
|
Yes |
This PR picks up the work done by @bradennapier in #7185, rebasing his work to the master branch and adding a few changes for to support
<PROJECT_ROOT>.Still to do:
module.system.node.resolve_dirnameif this touches code related to thatHandle the relative paths from the initial PRI'm almost inclined to not tackle this one, since<PROJECT_ROOT>will be handled. Thoughts?Note that this is the first time I've gone through this codebase and also my first time writing OCaml. Please nitpick away! Not sure if I'm following all of the right style conventions.