Improve handling of venvs and site-packages#3
Improve handling of venvs and site-packages#3msullivan wants to merge 4 commits intofacebook:mainfrom
Conversation
|
Hi @msullivan! Thank you for your pull request. We require contributors to sign our Contributor License Agreement, and yours needs attention. You currently have a record in our system, but the CLA is no longer valid, and will need to be resubmitted. ProcessIn order for us to review and merge your suggested changes, please sign 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 to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
msullivan
force-pushed
the
site_packages-venv
branch
from
bf08034 to
86d601b
Compare
|
Hi! I'll hold off on the review until the CLA is in. |
* Skip descending into directories/files with names that are invalid python modules. (Prevents descending into `venv/.../site-packages/...`) * When looking for the prefix of a module, check site-packages *before* the project root, since a `venv` will often live under the project root, and so currently it gets picked up first. * Support configuring site_packages with a command-line argument
msullivan
force-pushed
the
site_packages-venv
branch
from
86d601b to
6dd284a
Compare
|
How would you feel about a follow-up that make run-tree use |
|
@brittanyrey has imported this pull request. If you are a Meta employee, you can view this in D101645198. |
brittanyrey
left a comment
brittanyrey
left a comment
There was a problem hiding this comment.
overall lgtm! small changes/questions
| /target | ||
| *~ |
There was a problem hiding this comment.
why these two changes?
There was a problem hiding this comment.
/target/ - I think is definitely necessary - it is the default build output directory
*~ - This one is for me in particular, since *~ is my editor's backup files. (It's a pretty common .gitignore line; I could include some other editors too if we want to be more general)
I can put this in a separate PR though if you want
There was a problem hiding this comment.
This is fine to bundle! Thanks for the explanation.
|
@brittanyrey merged this pull request in 9b07b17. |
2 participants
python modules. (Prevents descending into
venv/.../site-packages/...)before the project root, since a
venvwill often liveunder the project root, and so currently it gets picked up first.