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

Updated Installation Doc for Python - Linux, Windows #435

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

jayam04
Copy link
Collaborator

@jayam04 jayam04 commented Jan 15, 2025

Python Installation Linux:

  1. Updated build dependencies for pyenv.
  2. Added serial numbers to headings.
  3. Added description for pyenv and direnv

Python Installation Windows:

  1. Added brief overview of installation methods.

@jayam04 jayam04 requested a review from seanlip January 15, 2025 18:41
Copy link
Member

@seanlip seanlip left a comment

Choose a reason for hiding this comment

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

Thanks @jayam04, these are great improvements! I made some comments, PTAL when you get a chance.


1. Create a new, empty folder that will hold your Oppia work. Here, we call the folder `opensource`.

2. Navigate to the folder (`cd opensource/`). Next, we'll [fork and clone](https://help.github.com/articles/fork-a-repo/) the Oppia repository.
2. Navigate to the folder (`cd opensource/`). Next, we'll [fork and clone](https://help.github.com/articles/fork-a-repo/) the Oppia repository, as described in below steps.
Copy link
Member

Choose a reason for hiding this comment

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

...in the steps below.

(Or just drop the 2nd sentence here and, in step 3, you can say "click on the fork button to fork Oppia's repository", where "fork Oppia's repository" is linked to https://help.github.com/articles/fork-a-repo/ .)


*These installation instructions were last tested on 8 January 2024. For more information on issues that may occasionally arise with the installation process, see the [Troubleshooting](https://github.com/oppia/oppia/wiki/Troubleshooting) page or ask in the [GitHub Discussions](https://github.com/oppia/oppia/discussions).*
> [!NOTE]
> These installation instructions were last tested on 8 January 2024. For more information on issues that may occasionally arise with the installation process, see the [Troubleshooting](https://github.com/oppia/oppia/wiki/Troubleshooting) page or ask in the [GitHub Discussions](https://github.com/oppia/oppia/discussions).
Copy link
Member

Choose a reason for hiding this comment

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

nit: ask in GitHub Discussions (drop "the")

> The commands below can be executed in any directory, as they are not path-specific.
1. **Make sure you install the Python build dependencies for your operating system. These are specified [here](https://github.com/pyenv/pyenv/wiki#suggested-build-environment). If you don't do this it might lead to problems further on.** The build dependencies for Ubuntu/Debian are

> [NOTE!]
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is working correctly, see the rich diff preview.

```

2. Install pyenv:
2. Install pyenv using [installation steps](https://github.com/pyenv/pyenv?tab=readme-ov-file#installation).
Copy link
Member

Choose a reason for hiding this comment

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

... using these installation steps: [link]

Or just say "Install pyenv." and link the whole text to that link.

@@ -119,7 +125,8 @@ For your virtual environment, we recommend you use [pyenv](https://github.com/py
...
```

If you see the warning at the end, add the following lines to your ``>> ~/.bashrc`` (if you are using bash) or ``>> ~/.zshrc`` (if you are using zsh).
> [!WARNING]
Copy link
Member

Choose a reason for hiding this comment

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

These don't seem to be showing up properly in the rich text diff. In future, please do a preview check before submitting for review.

@@ -182,7 +192,7 @@ exec "$SHELL"
> [!WARNING]
> Be careful with using graphical editors like Notepad in Windows. These can add carriage returns (`\r`) that confuse our Linux-based development tools. Instead, we recommend using editors designed for programming or command-line text editors.

8. Create a virtual environment for oppia by adding file named `.envrc` into the parent folder of the oppia repository
8. Create a virtual environment for oppia by adding file named `.envrc` into the parent folder of the oppia repository, which is `opensource/` in this case.
Copy link
Member

Choose a reason for hiding this comment

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

Create a virtual environment for Oppia by adding an .envrc file into the parent folder of the oppia repository (opensource/ in this case). The file should have the following content:

@@ -1,5 +1,11 @@
# Table of Contents

> [!NOTE]
> Most of scripts in Oppia are designed to support UNIX based systems, this includes Linix and MacOS but not Windows. So, for Windows we have 3 major methods to support this.
Copy link
Member

Choose a reason for hiding this comment

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

Most of scripts --> Most scripts

UNIX based systems, this includes --> UNIX-based systems. These include

Linix --> Linux

Also, what does "this" at the end refer to? It's not quite clear, perhaps worth rephrasing. Maybe "to support running these scripts:"?

@@ -1,5 +1,11 @@
# Table of Contents

> [!NOTE]
> Most of scripts in Oppia are designed to support UNIX based systems, this includes Linix and MacOS but not Windows. So, for Windows we have 3 major methods to support this.
> 1. Using Windows Subsystem for Linux 2 (WSL2) (recommended): It's provided by Microsoft and uses lightweight virtual machine with an actual Linux kernel.
Copy link
Member

Choose a reason for hiding this comment

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

uses --> uses a

> [!NOTE]
> Most of scripts in Oppia are designed to support UNIX based systems, this includes Linix and MacOS but not Windows. So, for Windows we have 3 major methods to support this.
> 1. Using Windows Subsystem for Linux 2 (WSL2) (recommended): It's provided by Microsoft and uses lightweight virtual machine with an actual Linux kernel.
> 2. Using Virtual Machine solutions such as Virtualbox, VMWare: It's easier to setup and you will have true Linux environment. However, it adds some overhead and reduces performance.
Copy link
Member

Choose a reason for hiding this comment

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

It's --> These are

have true --> have a true

> Most of scripts in Oppia are designed to support UNIX based systems, this includes Linix and MacOS but not Windows. So, for Windows we have 3 major methods to support this.
> 1. Using Windows Subsystem for Linux 2 (WSL2) (recommended): It's provided by Microsoft and uses lightweight virtual machine with an actual Linux kernel.
> 2. Using Virtual Machine solutions such as Virtualbox, VMWare: It's easier to setup and you will have true Linux environment. However, it adds some overhead and reduces performance.
> 3. Using Docker (caution: in BETA): It's a modern way which packages dependencies and utilizes OS-level virtualization rather than hardware-level. Currently, being in BETA, it still requires WSL, so it's no better than first method.
Copy link
Member

Choose a reason for hiding this comment

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

first --> the first

@jayam04 jayam04 changed the title Updated Installation Doc for Python - Linux Updated Installation Doc for Python - Linux, Windows Jan 16, 2025
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.

2 participants