-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@anisha1234 Thanks for your PR. I have a few suggestions for changes below.
README.md
Outdated
@@ -36,10 +36,11 @@ To install the frontend please do the following: | |||
3. Install [ember-cli latest](https://www.npmjs.org/package/ember-cli): `npm install -g ember-cli@latest`. | |||
Depending on your [npm permissions](https://docs.npmjs.com/getting-started/fixing-npm-permissions) you might need root access to install ember-cli. | |||
4. Install [bower](https://www.npmjs.org/package/bower): `npm install -g bower` | |||
5. Clone this repo with `git clone https://github.com/HospitalRun/hospitalrun-frontend`, go to the cloned folder and run `script/bootstrap`. | |||
5. Fork the master repository from `https://github.com/HospitalRun/hospitalrun-frontend` and clone the forked repository. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure why the change here. For some folks cloning is all they will need to do.
README.md
Outdated
@@ -75,13 +76,16 @@ To run HospitalRun with Docker please do the following: | |||
- Build the HospitalRun image with `docker build -t hospitalrun-frontend .` | |||
- Execute `docker run -it --name couchdb -d couchdb` to create the couchdb container. | |||
- Execute `docker run -it --name hospitalrun-frontend -p 4200:4200 --link couchdb:couchdb -d hospitalrun-frontend` to create the HospitalRun container. | |||
or instead, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The way this is formatted it isn't immediately clear that it is a different way to accomplish Running with Docker. I suggest breaking the Running With Docker into two sub sections, eg:
##Running With Docker
###Running With Docker Engine
Existing information here
###Running with Docker Compose
Your new info here
@jkleinsc I have changed the doc according to your suggestion.Kindly have a look at it |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Thanks for the PR @anisha1234!
Fixes #989
Changes proposed in this pull request:
cc @jkleinsc @jglovier @tangollama