Fix #1149 Correctly expand ~ in virtualenvs path#1158
Fix #1149 Correctly expand ~ in virtualenvs path#1158valignatev wants to merge 1 commit intopython-poetry:masterfrom
Conversation
Fix the bug reported in python-poetry#1149 Cover virtualenvs.path setting resolving with basic tests
|
Fixed merge conflicts in |
|
@sdispater for your consideration. I'm not sure whether this fits "bug" or "feature" better. |
|
I personally don't think that it's a feature, but if you think it is - should I retarget my PR against develop branch then? |
|
If this had ever worked before, or was documented to behave this way it would be clearly a bug. But I don't see that anywhere. |
|
In this sense, I agree. I don't think that the current behavior is expected by most of the users though. |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
Do you still interested in this PR? If so, I can fix the conflicts. |
|
I've removed the bot label but inclusion is for @sdispater to decide |
|
Hey @valignatev, do you think you can make your changes work with the current changes in the branch? Thanks a lot for your contribution! 👍 fin swimmer |
|
Sure, will do |
|
Hi, are you still interested in bringing this to Poetry? |
|
It's been 700 years, and I'm not really writing that much python these days let alone using poetry :) But you (or anybody else) can of course feel free to use anything form this PR for any purpose |
This is understandable. Thank you for your contribution anyway. I will probably use some of your code to recreate this in new PR :D |
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Fixes: #1149
I decided to extract virtualenvs path handling into the separate function so it could be easily tested.
I've tried more of an integrated approach locally, i.e., I've mocked
Env.build_venvto ensure it's called with the absolute path in case of~/settings, but it looked clumsy and didn't give an opportunity to cover more cases.I hope it'll help.