-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Update env remove logic #6195
Update env remove logic #6195
Conversation
Added venv_name fixture to tests/utils
Add IncorrectEnvError exception Add check_env_is_for_current_project method Add a couple of tests for new and old env remove logic
Update remove handling, so that if other project's venv name is passed, raise IncorrectEnvError for better error message
I think this code could use a general refactor/cleanup to make it clear we now support 'identifiers' that can be Python version numbers, Python executable paths, or environment names (as well as maybe a docs pass). If you'd like to do that, it would be great -- if not, this is still incremental progress. |
@neersighted yeah, I agree this could be a bit clearer in the code. I'd like to try to clean this up, wondering if there's any plan/ideas for it right now? |
Personally I would rework the code to actually be a simple |
If you're interested in working in this area still, #2748 might provide some more ideas for improvements to the CLI experience. |
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. |
Pull Request Check List
Resolves: #6018
Changes
python
argument is a file (then it should be a python path) - extract it's venv name and raiseIncorrectEnvError
if it doesn't belong to this projectBefore
After
Before
After
venv_name
fixture fortests/utils
directory to use intest_env
. Also replaced some of"simple-project"
hardcoded value to usepoetry.package.name
It's up to maintainers to choose what they want for this project - I'm happy either way if we at least fix the bug. I can remove/change any of the stuff I added on top of the fix. But yeah I just decided that if we fix the bug, we might also make some improvements/changes in this area of code. Any thoughts on this are welcome, thanks!