Skip to content
This repository has been archived by the owner on Jan 26, 2021. It is now read-only.

Fix: Minor updates to Setup documentation on ReadMe #570

Closed
wants to merge 2 commits into from

Conversation

LaibaBasit008
Copy link

@LaibaBasit008 LaibaBasit008 commented Jun 27, 2020

Description

Changes have been made in ReadMe document to improve the environment setup process for both Windows and Linux
-Correctly Numbered the Steps
-dev.py path has been updates
-python version was corrected
-the command to find path/to/python has been added

Fixes #563

Type of Change:

  • Documentation

Code/Quality Assurance Only

  • This change requires a documentation update (upgrade on readme file)

Checklist:

  • My PR follows the style guidelines of this project
  • I have commented relevant documentation, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation

@LaibaBasit008 LaibaBasit008 added the Status: Needs Review PR needs an additional review or a maintainer's review. label Jun 28, 2020
theyashshahs
theyashshahs previously approved these changes Jul 4, 2020
@theyashshahs
Copy link
Contributor

@LaibaBasit008 change the PR title to follow the commit styles :)

README.md Show resolved Hide resolved
@theyashshahs
Copy link
Contributor

ping @LaibaBasit008

@LaibaBasit008 LaibaBasit008 changed the title Changed Readme.md Minor updates to Setup documentation on ReadMe Jul 15, 2020
SanketDG
SanketDG previously approved these changes Jul 17, 2020
Copy link
Contributor

@SanketDG SanketDG left a comment

Choose a reason for hiding this comment

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

@LaibaBasit008 Changes look good! This should be ready to merge once the conflicts are resolved.

Make sure to follow our https://github.com/anitab-org/portal/wiki/Commit-Message-Style-Guide, and also please expand on what you did in this PR, not just "Updated README.md"

@LaibaBasit008 LaibaBasit008 changed the title Minor updates to Setup documentation on ReadMe Fix: Minor updates to Setup documentation on ReadMe Jul 17, 2020
@SanketDG
Copy link
Contributor

@LaibaBasit008 The update message is not yet committed!

@LaibaBasit008
Copy link
Author

@LaibaBasit008 The update message is not yet committed!

I am sorry I didn't fully understand!

@SanketDG
Copy link
Contributor

The update message is not yet committed!

I am sorry, I meant the committed message is not updated!

If you see the first commit in the "Commits" tab, it's not updated to follow the Commit Message Guidelines.

@LaibaBasit008
Copy link
Author

The update message is not yet committed!

I am sorry, I meant the committed message is not updated!

If you see the first commit in the "Commits" tab, it's not updated to follow the Commit Message Guidelines.

updated!

@SanketDG
Copy link
Contributor

@LaibaBasit008 You now have an extra commit, and the "Updated Readme.md" commit is not yet updated to follow the guidelines.

Please ping on Zulip, if you need help.

sakshi1499
sakshi1499 previously approved these changes Aug 3, 2020
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Copy link

@sakshi1499 sakshi1499 left a comment

Choose a reason for hiding this comment

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

LGTM

you still get an error due to proxy, use "-E" flag along with "sudo" to export all the
environment variables.
1. Make sure you have python3-dev installed on your operating system. For Debian, you would additionally require libpq-dev.
3. Make sure you have python3-dev installed on your operating system. For Debian, you would additionally require libpq-dev.
Install by using `sudo apt-get install libpq-dev python3-dev`
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this package libpq-dev python3-dev, because I have noticed while working with GitHub actions's setup it is not required (or not addtionally required for adding this package) see.

Copy link
Contributor

Choose a reason for hiding this comment

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

I definitely think we need this because psycopg2 is a C-extension module, so it needs the header files for linking.

Are you sure about this?

Choose a reason for hiding this comment

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

I think we need this 🙄

Choose a reason for hiding this comment

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

the `portal` directory. If working behind a proxy, follow the instructions [here](https://cms-sw.github.io/tutorial-proxy.html).
1. Create a virtual environment with Python 3 and install dependencies:
6. Create a virtual environment with Python 3.6 and install dependencies:
Copy link
Contributor

Choose a reason for hiding this comment

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

Users can create virtual env using python 3.7 also(Update need here).

Copy link
Contributor

Choose a reason for hiding this comment

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

Will update this

@@ -66,7 +69,7 @@ If you face some issues while installing and making Portal up in your local, hav
Setup for developers (Windows)
------------------------------

1. Make sure you have installed Python 3.6, make sure you the right one (32/64 bits). [Source](https://www.python.org/downloads/). During installation please pay attention to the following details :
1. Make sure you have installed Python 3.6 or 3.7, make sure you the right one (32/64 bits). [Source](https://www.python.org/downloads/). During installation please pay attention to the following details :
- Tick/Select Add Python 3.6 to PATH
Copy link
Contributor

Choose a reason for hiding this comment

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

We can add here some detail here like if you choose Python 3.6 then Do this else Do that.

Copy link
Contributor

@SanketDG SanketDG Aug 6, 2020

Choose a reason for hiding this comment

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

Do you mean to expand on the PATH instructions?

Choose a reason for hiding this comment

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

I think Satya wants one or two line on Path only :D

@sakshi1499
Copy link

@SanketDG Are you pushing the changes in this or Laiba?

@sakshi1499 sakshi1499 added Status: Changes Requested Changes are required to be done by the PR author. and removed Status: Needs Review PR needs an additional review or a maintainer's review. labels Aug 13, 2020
@sakshi1499
Copy link

There are conflicts, some changes have to be made too. Closing this PR as there is no activity.

@sakshi1499 sakshi1499 closed this Sep 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Status: Changes Requested Changes are required to be done by the PR author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Docs: Minor updates to Setup documentation on ReadMe
6 participants