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 PIPX_HOME move instruction and improve text #1063

Merged
merged 2 commits into from
Nov 30, 2023
Merged

Fix PIPX_HOME move instruction and improve text #1063

merged 2 commits into from
Nov 30, 2023

Conversation

bulletmark
Copy link
Contributor

The procedure previously documented in the section I changed here was incorrect because it assumes all packages were originally from PyPi, so does not work for packages installed from local source, or packages installed from VCS sources. I fixed the procedure to simply use pipx reinstall. I also found the section was a little confusing so I have rewritten t to hopefully make it clearer.

Copy link
Contributor

@chrysle chrysle left a comment

Choose a reason for hiding this comment

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

A few rephrasing suggestions.

the old path to the new paths, you can move the `~/.local/pipx`
directory to the new location and then reinstall all packages. For
example, on Linux systems, `PIPX_HOME` moves from `~/.local/pipx` to
`~/.local/share/pipx` so do this:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
`~/.local/share/pipx` so do this:
`~/.local/share/pipx`, so one can do this:

Comment on lines 141 to 142
mkdir -p ~/.local/share
mv ~/.local/pipx ~/.local/share/
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems more comfortable.

Suggested change
mkdir -p ~/.local/share
mv ~/.local/pipx ~/.local/share/
mkdir -p ~/.local/share && mv ~/.local/pipx ~/.local/share/

Comment on lines 134 to 135
the old path to the new paths, you can move the `~/.local/pipx`
directory to the new location and then reinstall all packages. For
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
the old path to the new paths, you can move the `~/.local/pipx`
directory to the new location and then reinstall all packages. For
the old path to the new paths, you can move the `~/.local/pipx`
directory to the new location (after removing cache, log and trash directories which will be moved automatically) and then reinstall all packages. For

Comment on lines 127 to 128
Version 1.2.0 and earlier use `PIPX_HOME=~/.local/pipx` and locate the
data, cache, and log directories under that `PIPX_HOME`. To maintain
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Version 1.2.0 and earlier use `PIPX_HOME=~/.local/pipx` and locate the
data, cache, and log directories under that `PIPX_HOME`. To maintain
Version 1.2.0 and earlier use `~/.local/pipx` as the default `PIPX_HOME` and install the
data, cache, and log directories under it. To maintain

@bulletmark
Copy link
Contributor Author

@chrysle thanks. I adopted your suggestions basically as is except instead of your

after removing cache, log and trash directories which will be moved automatically

I wrote:

after removing cache, log and trash directories which will get recreated automatically

which I think is more accurate.

@uranusjr
Copy link
Member

uranusjr commented Oct 5, 2023

Please rebase to or merge latest main for CI. The PR does not enable committer access so I can’t do it for you.

@bulletmark
Copy link
Contributor Author

@uranusjr I wasn't sure how to do this so ended up force-pushing my rebased branch (although it does seem to have worked?) BTW, you say "The PR does not enable committer access so I can’t do it for you" but I have the "allow edits by maintainers" option set on this PR. Are you talking about something else?

@uranusjr
Copy link
Member

uranusjr commented Oct 6, 2023

Hmm, not sure, GitHub did not allow me to resolve conflicts and I assumed it’s due to the permission setting. Maybe it was caused some other glitch.

@uranusjr
Copy link
Member

uranusjr commented Oct 6, 2023

Can you check the file’s history to find out what changed between when you first opened the PR and force pushed?

@bulletmark
Copy link
Contributor Author

@uranusjr Note I did rebase correctly against the newly fetched main in my local repo but because my branch on github was not rebased then I had to force push it to github. So the commits here are correct. I only changed 1 doc file. The first commit is my original change, same as original PR then the second commit is my response to chrysle comments.

Copy link
Contributor

@gaborbernat gaborbernat left a comment

Choose a reason for hiding this comment

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

Can you fix the merge conflicts?

@gaborbernat gaborbernat merged commit 18d6722 into pypa:main Nov 30, 2023
4 of 10 checks passed
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.

None yet

4 participants