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

updated readme #748

Merged
merged 4 commits into from
Oct 17, 2016
Merged

updated readme #748

merged 4 commits into from
Oct 17, 2016

Conversation

mdarmanin
Copy link
Contributor

@mdarmanin mdarmanin commented Oct 17, 2016

Fixes #[replace brackets with the issue number that your pull request addresses].

Changes proposed in this pull request:

  • Updated the Readme.md to include an important step in the setup process
  • Another small touchup to the readme.md, them blasted typos.

cc @HospitalRun/core-maintainers

@jkleinsc
Copy link
Member

@mdarmani thanks for the PR! Can you update the description of what you changed? I know this is a simple change, but it really helps us a project maintainers to understand what PRs are intended for. I specifically mean this text below should be replaced. If your PR doesn't fix an issue you can drop that part out, but there should be some kind of description of the change.
Fixes #[replace brackets with the issue number that your pull request addresses].

Changes proposed in this pull request:

  • [list out summary of changes here]
  • [list out summary of changes here]
  • [list out summary of changes here]
  • [etc]

Note: pull requests without proper descriptions may simply be closed without further discussion. We appreciate your contributions, but need to know what you are offering in clearly described format. Thanks! (you can delete this text)

Copy link
Member

@jkleinsc jkleinsc left a comment

Choose a reason for hiding this comment

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

Thanks for the addition, I think the text should be slightly tweaked as suggested below:

@@ -48,7 +48,7 @@ To install the frontend please do the following:
## Start
To start the frontend please do the following:

- Start the server by running `npm start` in the repo folder.
- Start the server by running `npm start` in the repo folder. In some cases, that will not work so try `ember serve`.
Copy link
Member

Choose a reason for hiding this comment

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

Saying "In some cases" is kind of confusing. Maybe you could say something like, if npm start doesn't work you you, you can try ember serve as an alternative?

Copy link
Member

@jkleinsc jkleinsc left a comment

Choose a reason for hiding this comment

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

Just needs a slight grammar tweak.

@@ -48,7 +48,7 @@ To install the frontend please do the following:
## Start
To start the frontend please do the following:

- Start the server by running `npm start` in the repo folder.
- Start the server by running `npm start` in the repo folder. If `npm start` doesn't work you, try `ember serve` as an alternative.
Copy link
Member

Choose a reason for hiding this comment

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

I think you meant to say "If npm start doesn't work for you

@jkleinsc
Copy link
Member

@mdarmani Looks good to me. Thanks for the PR! I'll merge it in.

@jkleinsc jkleinsc merged commit a48c2e7 into HospitalRun:master Oct 17, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants