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

Update installing-from-source.rst #135

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mindflowgo
Copy link

@mindflowgo mindflowgo commented Jul 16, 2024

Adjusting the paths for venv, zsh was wrong, I'm assuming other 2 require similar changes.


🚀 This description was created by Ellipsis for commit a4b5f16

Summary:

Updated virtual environment activation paths in src/installing-from-source.rst to use the home directory instead of the current directory.

Key points:

  • Updated src/installing-from-source.rst to change virtual environment activation paths.
  • For bash/zsh users, changed source ./venv/bin/activate to source ~/.venv/bin/activate.
  • For Windows git bash users, changed source ./venv/Scripts/activate to source ~/.venv/Scripts/activate.
  • For fish users, changed source ./venv/bin/activate.fish to source ~/.venv/bin/activate.fish.

Generated with ❤️ by ellipsis.dev

Adjusting the paths for venv, zsh was wrong, I'm assuming other 2 require similar changes.
Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

❌ Changes requested. Reviewed everything up to a4b5f16 in 57 seconds

More details
  • Looked at 20 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. src/installing-from-source.rst:76
  • Draft comment:
    The change in virtual environment activation paths from ./venv to ~/.venv could lead to confusion since the virtual environment is initially created in the local directory (python3 -m venv venv). Users following the guide might not have a ~/.venv directory, leading to errors when trying to activate the virtual environment.
    source ./venv/Scripts/activate
  • Reason this comment was not posted:
    Marked as duplicate.
2. src/installing-from-source.rst:78
  • Draft comment:
    The change in virtual environment activation paths from ./venv to ~/.venv could lead to confusion since the virtual environment is initially created in the local directory (python3 -m venv venv). Users following the guide might not have a ~/.venv directory, leading to errors when trying to activate the virtual environment.
    source ./venv/bin/activate.fish
  • Reason this comment was not posted:
    Marked as duplicate.

Workflow ID: wflow_N3YJ8g9S36iExBRx


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@@ -71,11 +71,11 @@ Now activate the virtualenv in your current shell session:
.. code-block:: sh

# For bash/zsh users:
source ./venv/bin/activate
source ~/.venv/bin/activate
Copy link

Choose a reason for hiding this comment

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

The change in virtual environment activation paths from ./venv to ~/.venv could lead to confusion since the virtual environment is initially created in the local directory (python3 -m venv venv). Users following the guide might not have a ~/.venv directory, leading to errors when trying to activate the virtual environment.

Suggested change
source ~/.venv/bin/activate
source ./venv/bin/activate

Copy link
Author

Choose a reason for hiding this comment

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

On the mac, it creates a directory caled '.venv' with the period to make it a hidden directory. Thus you could resolve the bot dispute to: source ./.venv/bin/activate

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.

1 participant