-
Notifications
You must be signed in to change notification settings - Fork 113
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
Update README.md #247
Update README.md #247
Conversation
@himedlooff @jimmynotjim @sebworks @juidai |
Make sure to fork this repo. You should be making a PR from your fork. Will review your updates. |
@@ -1,55 +1,70 @@ | |||
# cfgov-refresh | |||
|
|||
This repository contains the redesign-in-progress of consumerfinance.gov. This includes front-end assets and build tools, and configuration for [Sheer](https://github.com/cfpb/sheer) to load content from Wordpress and Django back-ends to elasticsearch to render the site. | |||
**Description**: This repository contains the redesign-in-progress of the [consumerfinance.gov](http://consumerfinance.gov) website. This includes front-end assets and build tools, and configuration for [Sheer](https://github.com/cfpb/sheer) to load content from WordPress and Django back-ends to Elasticsearch to render the site. |
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.
Re: "Description": some of these bold labels in the open source template are meant as guides and can be left out.
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.
Thinking about it, This repository contains
could probably be left out also.
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.
👍
Wow, THANK YOU for adding in these notes. |
And thank you for the review!! I'll get these in tomorrow. Happy to open a new PR from a fork too if needed for this. Is that just to prevent accidental writes to the master branch? |
Not sure, @Scotchester, why do we fork instead of using branches on the origin repo? |
@himedlooff Think I got them all. For "tech stack"/"dependencies" I'll leave as-is and open an issue on the template. Before merging let me know if it's good to go and I'll squash my commits together. |
@@ -1,55 +1,82 @@ | |||
# cfgov-refresh | |||
|
|||
This repository contains the redesign-in-progress of consumerfinance.gov. This includes front-end assets and build tools, and configuration for [Sheer](https://github.com/cfpb/sheer) to load content from Wordpress and Django back-ends to elasticsearch to render the site. | |||
The redesign-in-progress of the [consumerfinance.gov](http://consumerfinance.gov) website. |
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.
Would this be better as "The in-progress redesign..."?
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.
👍
|
||
## Getting started | ||
```bash | ||
$ mkvirtualenv cfgov-refresh |
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.
Update all bash commands to either include $
or not for better consistency.
$ elasticsearch --config=/Users/<YOUR MAC OSX USERNAME>/homebrew/opt/elasticsearch/config/elasticsearch.yml | ||
``` | ||
|
||
### 3. Launch sheer to serve the site |
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.
"Sheer" when used as the project name should be uppercase here and throughout
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.
👍
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.
Fixed in 52447c7
- Fixes #247 (comment), #247 (comment) - Adds missing `$` in terminal code snippets.
2. In _tests/browser_testing/features/, copy example-environment.cfg to | ||
environment.cfg and change the `chrome_driver` path to the proper path | ||
for your webdriver binary. If you installed via homebrew, | ||
this will be /path/to/homebrew/bin/chromedriver. |
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 Homebrew path should be an inline codeblock
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.
👍 That contains a lot that should be inline codeblock actually.
Looks good to me. I'll let @jimmynotjim merge since he was the last to comment. |
#### 1. Back-end setup | ||
|
||
Follow the [Sheer installation instructions](https://github.com/cfpb/sheer#installation) | ||
to get sheer installed. |
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.
One more instance of lower "sheer"
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.
👍
@himedlooff @jimmynotjim Don't merge this, just let me know it's all set. I'll squash all my "code review edit" commits and merge. |
Ans, squash this down and I'll merge it. |
- Adds Readme structure inspired by CFPB project template at https://github.com/cfpb/open-source-project-template. - Adds more comprehensive installation instructions (of note, there were no instructions for starting Elasticsearch). - Removes copy that talks about how Wordpress was used in the project in the past. - Removes copy that references GitHub Enterprise. - Cleans up unneeded tab characters. - Moves the working with templates section under the Getting Involved section. Additionally fixes: #247 (comment), #247 (comment), #247 (comment), #247 (comment), #247 (comment), #247 (comment), #247 (comment), #247 (comment) - Adds missing `$` in terminal code snippets. More additional fixes: #247 (comment), #247 (comment), #247 (comment), #247 (comment)
a32a1dd
to
960b369
Compare
- Adds Readme structure inspired by CFPB project template at https://github.com/cfpb/open-source-project-template. - Adds more comprehensive installation instructions (of note, there were no instructions for starting Elasticsearch). - Removes copy that talks about how Wordpress was used in the project in the past. - Removes copy that references GitHub Enterprise. - Cleans up unneeded tab characters. - Moves the working with templates section under the Getting Involved section. Additionally fixes: #247 (comment), #247 (comment), #247 (comment), #247 (comment), #247 (comment), #247 (comment), #247 (comment), #247 (comment) - Adds missing `$` in terminal code snippets. More additional fixes: #247 (comment), #247 (comment), #247 (comment), #247 (comment)
https://github.com/cfpb/open-source-project-template.
were no instructions for starting Elasticsearch).
in the past.
section.