Skip to content

Improve --devenv help and ensure --devenv does not delete #1333

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

Merged
merged 1 commit into from
Jun 2, 2019
Merged

Improve --devenv help and ensure --devenv does not delete #1333

merged 1 commit into from
Jun 2, 2019

Conversation

asottile
Copy link
Contributor

@asottile asottile commented Jun 1, 2019

No description provided.

@asottile asottile mentioned this pull request Jun 1, 2019
6 tasks
@obestwalter
Copy link
Member

I don't quite get what this is supposed to do.

Calling tox --devenv already failed in the last version as it needs an argument. If I call tox --devenv . this should still delete the project I guess or doesn't it?

So my comment mentioning the problem with envdir wasn't about accidentally the whole project. It was about breaking the promise that venvs end up in .tox by default. Overwriting arbitrary folders with a an unqualified venv name is still perfectly possible, isn't it?

@obestwalter
Copy link
Member

obestwalter commented Jun 1, 2019

I thought about this again. For you having the venv in the project root is a feature so you want this to be the default. I understand that. For me this is a potential danger, because I am giving courses for beginners now and then and I see the footgun potential. This is the conflict and I would be interested to hear your thoughts on that. To me people like you and @gaborbernat are the prototypical power users who know their tools in and out (and they also reprogram them if they don't do what they should), but maybe sometimes you forget that the average user has not the kind of insight like you have?

@gaborbernat
Copy link
Member

I actually think exposing development environments at root level is what beginners expect, in my experience with my other colleagues.

@obestwalter
Copy link
Member

My experience is more along the lines of people not even having a clue yet what a venv is. So again there might be some slight bias here. Not all of us are deeply embedded in the Python ecosystem working in companies together with Python core devs (say hello to @pablogsal for me if you bump into him) :D

@gaborbernat
Copy link
Member

My experience is actually not based on core devs,just average python developers.

@asottile
Copy link
Contributor Author

asottile commented Jun 2, 2019

I don't quite get what this is supposed to do.

Calling tox --devenv already failed in the last version as it needs an argument. If I call tox --devenv . this should still delete the project I guess or doesn't it?

In the previous version tox --devenv '' (the empty string as an argument) would act as if --devenv wasn't passed at all, but it should have acted as if devenv was passed as the empty string (which should trigger an error about deleting the project). tox --devenv . also now triggers the same error 👍

@obestwalter
Copy link
Member

Yeah ... IMO it's already treating symptoms we wouldn't have if we'd cure the illness though. See #1326 (comment)

@gaborbernat gaborbernat merged commit bed76e6 into tox-dev:master Jun 2, 2019
@asottile asottile deleted the devenv_empty_string branch June 2, 2019 14:46
@pablogsal
Copy link

(say hello to @pablogsal for me if you bump into him) :D

Hi @obestwalter ✋ :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants