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

Check install path #6801

Closed
wants to merge 7 commits into from
Closed

Conversation

tfolbrecht
Copy link

Enhancement proposal:

issue #6762

Before

if a user runs pip install without write access to the target directory, it will collect and download packages before failing with permission denied.

Before

After

This PR uses the check_path_owner function from pip._internal.utils.filesystem to check write permissions and exits with an error message

Screenshot from 2019-07-28 10-22-10

I'm still learning python.
If it would be better to move the option checks to a function I'll close this and go ahead and do that.

Thank you for taking the time to review my PR.

@pradyunsg
Copy link
Member

@tfolbrecht You don't need to close the PR -- you can just push more commits to the same branch. :)

@pradyunsg
Copy link
Member

pradyunsg commented Jul 28, 2019

Here is an old, but relevant article about pull requests: https://yangsu.github.io/pull-request-tutorial/ :)

@tfolbrecht
Copy link
Author

Thank you for the article, I kind of paniced 😄

@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 Aug 12, 2019
@tfolbrecht
Copy link
Author

Thank you Mr Bot!
I'll follow up.

@pypa-bot pypa-bot removed the needs rebase or merge PR has conflicts with current master label Sep 23, 2019
@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 28, 2020
@McSinyx
Copy link
Contributor

McSinyx commented Mar 3, 2020

What is the status of this @tfolbrecht? May I take this over (basically appending stylistic changes over to satisfy the checks) or do you want to do it?

@tfolbrecht
Copy link
Author

I abandoned it @McSinyx. Go for it! I may try again if you don't and I get back into the python ecosystem.

@tfolbrecht
Copy link
Author

PR was adopted :) #7828

@tfolbrecht tfolbrecht closed this Apr 14, 2020
@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.
Labels
auto-locked Outdated issues that have been locked by automation needs rebase or merge PR has conflicts with current master
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants