Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Drop --use-venv from all CI tests #7999

Closed
wants to merge 1 commit into from
Closed

Drop --use-venv from all CI tests #7999

wants to merge 1 commit into from

Conversation

pradyunsg
Copy link
Member

Toward #7997 (comment)

This is being done since our test setup is basically borked with venv, and until that is fixed, I think those tests cause more issues than they add significant value (see #7718 and #7356 for context).

@pradyunsg
Drop --use-venv from all CI tests
1d09972 
This is being done since our test setup is basically borked with venv.
@pradyunsg pradyunsg added skip news Does not need a NEWS file entry (eg: trivial changes) state: needs discussion This needs some more discussion labels Apr 8, 2020
@xavfernandez
Copy link
Member

Since our current default venv_type is the legacy virtualenv, I'm lukewarm to the idea of dropping our tests on "modern" venv which should IMHO be the default.
Even more now that virtualenv 20 is now mimicking the venv mode...

If one mode should actually be dropped I'd argue that it should actually be the legacy virtualenv.

@BrownTruck
Copy link
Contributor

Hello!

I am an automated bot and I have noticed that this pull request is not currently able to be merged. If you are able to either merge the master branch into this pull request or rebase this pull request against master then it will be eligible for code review and hopefully merging!

@BrownTruck BrownTruck added the needs rebase or merge PR has conflicts with current master label Apr 13, 2020
@pradyunsg
Copy link
Member Author

pradyunsg commented Apr 13, 2020 •
edited
Loading

I did end up removing the bit that annoyed me (AP's use of useVenv that wasn't actually correct) in #8028, so I'm perfectly fine w/ closing this.

As for Travis CI, we can cross that bridge when it becomes an issue.

@pradyunsg pradyunsg closed this Apr 13, 2020
@pradyunsg pradyunsg deleted the ci/remove-use-venv branch April 13, 2020 14:37
@lock lock bot added the auto-locked Outdated issues that have been locked by automation label May 20, 2020
@lock lock bot locked as resolved and limited conversation to collaborators May 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
auto-locked Outdated issues that have been locked by automation needs rebase or merge PR has conflicts with current master skip news Does not need a NEWS file entry (eg: trivial changes) state: needs discussion This needs some more discussion
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pradyunsg @xavfernandez @BrownTruck