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

updated app setup in readme #200

Closed
wants to merge 3 commits into from
Closed

updated app setup in readme #200

wants to merge 3 commits into from

Conversation

plemiszki
Copy link
Contributor

Changes proposed in this pull request:

@afeld
Copy link
Contributor

afeld commented Jul 23, 2016

Going to defer to @jessieay or someone else more familiar with the project on this one. If Foreman is used in all environments, it seems that the entire line can be removed from CONTRIBUTING. If it's only used for development, I think it's fine for the project to be opinionated and say "run this project in development using Foreman", and just having Foreman in the development group of the Gemfile.

/cc #179

@plemiszki
Copy link
Contributor Author

Yes, that's probably true. I removed the line. The gem is used in production however, so I didn't move it to the development group.

@afeld
Copy link
Contributor

afeld commented Jul 23, 2016

Looks good to me! Will wait on @jessieay to merge.

@jessieay
Copy link
Contributor

@plemiszki you're right, foreman is used in prod. That comment in CONTRIBUTING is from a boilerplate rails app setup that didn't use foreman in production. Mind removing

If you don't have foreman, see Foreman's install instructions. It is purposefully excluded from the project's Gemfile.

while we're here?

thanks for the pR!

@plemiszki
Copy link
Contributor Author

@jessieay sorry for the delay. I have removed the comment. Please let me know if everything looks ok now.

@jessieay
Copy link
Contributor

jessieay commented Aug 2, 2016

there was a merge conflict so I pulled your branch down and made a few tweaks before merging. Merged at a2f6a0f - thank you very much!!

@jessieay jessieay closed this Aug 2, 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