FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

docs: in venv table use executable name by musvaage · Pull Request #124315 · python/cpython · GitHub

Repository navigation

docs: in venv table use executable name - #124315

Merged
vsajip merged 1 commit into
python:mainfrom
vaerksted:main
Oct 9, 2024
Merged

vsajip merged 1 commit into
python:mainfrom
vaerksted:main

Conversation

musvaage commented Sep 22, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

changes PowerShell to pwsh and pwsh.exe
changes POSIX PowerShell to pwsh


📚 Documentation preview 📚: https://cpython-previews--124315.org.readthedocs.build/

musvaage requested a review from vsajip as a code owner September 22, 2024 16:23
bedevere-app Bot added awaiting review docs Documentation in the Doc dir skip news labels Sep 22, 2024

vsajip commented Sep 23, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

On my Windows computer, there's no pwsh.exe - PowerShell is powershell.exe. So I'm not sure if your change should be applied as is - perhaps have powershell.exe or pwsh.exe replace PowerShell?

Copy link
Copy Markdown
Member

On my Windows computer, there's no pwsh.exe - PowerShell is powershell.exe. So I'm not sure if your change should be applied as is - perhaps have powershell.exe or pwsh.exe replace PowerShell?

I have both -- pwsh.exe is PowerShell 7 and powershell.exe is PowerShell 5 (Windows PowerShell). This appears to be official.

A

AA-Turner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

The change for POSIX seems fine but per Vinay's note the Windows entry should read either "PowerShell" or indicate that both powershell.exe and pwsh.exe are supported.

A

bedevere-app Bot commented Sep 23, 2024

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

Copy link
Copy Markdown
Contributor Author

Windows entry

unless something like the following is workable, I'll revert to the string PowerShell, and perform a soft reset

please clarify Member preference

Doc/library/venv.rst

  .. versionadded:: 3.8
     PowerShell activation scripts installed under POSIX for PowerShell Core
-    support.
+    support (users of PowerShell 5.1 replace pwsh.exe with powershell.exe in
+    the table).

musvaage commented Sep 25, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@AA-Turner

if neither the herein nor last comment's ideas are workable, I'll revert to the string PowerShell and perform a soft reset

in the last comment it was suggested the text in .. versionadded:: 3.8 could be supplemented

a different approach might be adding a Note: directly after the text of .. versionadded:: 3.8

     support.
  
+ .. note::
  
+     Users of PowerShell 5.1 replace pwsh.exe with powershell.exe in
+     the table.
+    
  You don't specifically *need* to activate a virtual environment,

musvaage marked this pull request as draft September 29, 2024 21:10
musvaage changed the title docs: in venv table use executable names docs: in venv table use executable name Sep 30, 2024

musvaage commented Sep 30, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

pwsh.exe
powershell.exe

I was disinclined to widen the table's second column.


Lib/venv/scripts/common/activate

CPython and PyPI packages' docs would be simpler if the bash/zsh script was named activate.sh.

python -m venv venv

Then it could be coded that resultant of the above command venv/bin/activate.sh was copied to venv/bin/activate.

That could remain in the code for some years until a notification was made on file copying deprecation.

regarding documentation simplification

At minimum then is a packager notice to the user to source or run the appropriate script for their preferred command-line shell.

musvaage marked this pull request as ready for review September 30, 2024 17:42
musvaage requested a review from AA-Turner September 30, 2024 17:43
vsajip merged commit 7f93dbf into python:main Oct 9, 2024
vsajip added needs backport to 3.12 only security fixes needs backport to 3.13 only security fixes labels Oct 9, 2024

Copy link
Copy Markdown

Thanks @musvaage for the PR, and @vsajip for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

Copy link
Copy Markdown

Thanks @musvaage for the PR, and @vsajip for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 9, 2024
(cherry picked from commit 7f93dbf)

Co-authored-by: musvaage <112724366+musvaage@users.noreply.github.com>
Co-authored-by: musvaage <musvaage@users.noreply.github.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 9, 2024
(cherry picked from commit 7f93dbf)

Co-authored-by: musvaage <112724366+musvaage@users.noreply.github.com>
Co-authored-by: musvaage <musvaage@users.noreply.github.com>

bedevere-app Bot commented Oct 9, 2024

Copy link
Copy Markdown

GH-125171 is a backport of this pull request to the 3.12 branch.

bedevere-app Bot removed the needs backport to 3.12 only security fixes label Oct 9, 2024

bedevere-app Bot commented Oct 9, 2024

Copy link
Copy Markdown

GH-125172 is a backport of this pull request to the 3.13 branch.

bedevere-app Bot removed the needs backport to 3.13 only security fixes label Oct 9, 2024
efimov-mikhail pushed a commit to efimov-mikhail/cpython that referenced this pull request Oct 9, 2024
Co-authored-by: musvaage <musvaage@users.noreply.github.com>
vsajip pushed a commit that referenced this pull request Oct 9, 2024
vsajip pushed a commit that referenced this pull request Oct 9, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Documentation in the Doc dir skip issue skip news

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants


Back | FazBrowse Home | New Git URL