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

tackle the manual a bit. #45

Merged
merged 7 commits into from
May 8, 2018
Merged

tackle the manual a bit. #45

merged 7 commits into from
May 8, 2018

Conversation

novski
Copy link
Contributor

@novski novski commented May 7, 2018

think it was because of the pipenv shell command that i used now that it worked so i add it to the manual... Also corrected a vim to editor line and split up the flow of point 12.

@coveralls
Copy link

coveralls commented May 7, 2018

Coverage Status

Coverage remained the same at 99.659% when pulling 4d60c7e on novski:master into e6d1205 on cdubz:master.

Copy link
Member

@cdubz cdubz left a comment

Choose a reason for hiding this comment

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

See individual review notes below. Thanks for you help clarifying these steps!

@@ -210,6 +210,7 @@ Python 3.x, nginx, uwsgi and sqlite and should be sufficient for a few users
1. Initiate the Python environment

pipenv install --three --dev
pipenv shell
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 necessary here. If you take a look at your own log in #43, note that pipenv shell is never used. It is the other pieces that got you over the hump, I believe. Adding this doesn't actually hurt anything, but I would prefer you remove it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i assume it is the bart that make it work because a frend told me on debian systems make differences when using sudo to user installations i think the environement keeps it in space.

README.md Outdated
`virtualenv` parameter with the command `pipenv --venv`.**

Symlink it and restart:
Copy link
Member

Choose a reason for hiding this comment

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

I like this idea! It does make the flow of the steps much cleaner. But, please change this to treat the "Symlink it and restart" commands as a new step. I think that will make it even cleaner.


sudo ln -s /etc/uwsgi/apps-available/babybuddy.ini /etc/uwsgi/apps-enabled/babybuddy.ini
sudo service uwsgi restart

1. Create and configure the nginx server
Copy link
Member

Choose a reason for hiding this comment

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

Please also separate these steps in the same way you did for the nginx config steps. Again, great idea!

@@ -180,7 +180,7 @@ Python 3.x, nginx, uwsgi and sqlite and should be sufficient for a few users

1. Install Python 3.x, pip, nginx and uwsgi

sudo apt-get install python3 python3-pip nginx uwsgi uwsgi-plugin-python3 git
sudo apt-get install python3 python3-pip nginx uwsgi uwsgi-plugin-python3 git libopenjp2-7-dev

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i think i forgot to add that dependency

Copy link
Contributor Author

@novski novski left a comment

Choose a reason for hiding this comment

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

i did.

@cdubz cdubz merged commit d4bd803 into babybuddy:master May 8, 2018
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.

3 participants