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.

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

fix(_internal/cli/cmdoptions): Add --require-virutalenv to docs #8251

Closed

Conversation

gutsytechster
Copy link
Contributor

@gutsytechster gutsytechster commented May 16, 2020

Towards #2429

@gutsytechster
Copy link
Contributor Author

gutsytechster commented May 16, 2020

I was thinking to add a functional test to test this option. But script fixture runs the pip in the virtual environment. I couldn't think of the way to test the absence of the virtual environment.
Let me know if someone finds a way to do so.

Copy link
Member

@pradyunsg pradyunsg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs a documentation section in the User Guide, describing what it does and how to use pip config to set this to True.

@pradyunsg pradyunsg added S: awaiting response Waiting for a response/more information type: feature request Request for a new feature labels May 16, 2020
@deveshks
Copy link
Contributor

deveshks commented May 17, 2020

I might be wrong, but I think that fixes #2429 in the PR description will close the original issue, which isn't right since the issue also tracks other hidden options (as per #2429 (comment)). I think you can change the title to Towards #2429 perhaps?

@gutsytechster gutsytechster force-pushed the add_docs_require_virtualenv branch from 9325de4 to 5eba32f Compare May 17, 2020 12:55
@gutsytechster gutsytechster requested a review from pradyunsg May 17, 2020 17:51
@gutsytechster gutsytechster force-pushed the add_docs_require_virtualenv branch from 5eba32f to 8fa1b88 Compare May 24, 2020 10:27
@gutsytechster
Copy link
Contributor Author

I think this can be merged now. :P

@pradyunsg pradyunsg removed the S: awaiting response Waiting for a response/more information label Jun 11, 2020
@pradyunsg
Copy link
Member

Not yet. The big fish of how do we test this behavior, is still left. :)

@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 Jun 27, 2020
@BrownTruck BrownTruck removed the needs rebase or merge PR has conflicts with current master label Jul 29, 2020
@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 Feb 23, 2021
@pradyunsg
Copy link
Member

Closing since this is out of date and bitrotten. Please feel free to file a new PR for this! ^>^

@pradyunsg pradyunsg closed this Apr 2, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs rebase or merge PR has conflicts with current master type: feature request Request for a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants