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.

Sign up for GitHub

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

Jump to bottom

Add templates #10

Merged
merged 6 commits into from
Mar 20, 2017
Merged

Add templates #10

merged 6 commits into from
Mar 20, 2017

Conversation

kariabancroft
Copy link
Contributor

  • Add feedback template
  • Add comprehension questions

Feedback requested:

  • Does the feedback template cover the important points? It seems like this one is more "they did it" since it's the first Rails project
  • Are the comprehension questions too in-depth? I'd like folks to have to spend some time trying to paraphrase the pieces of the request cycle early on to identify areas of concern

CheezItMan
CheezItMan approved these changes Mar 15, 2017
View reviewed changes
Copy link
Contributor

@CheezItMan CheezItMan left a comment

Choose a reason for hiding this comment

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

Definitely an improvement

.github/PULL_REQUEST_TEMPLATE Outdated
| :------------- | :------------- |
| Describe what the Model is doing in Rails | |
| Describe what the Controller is doing in Rails | |
| Describe what the View is doing in Rails | |
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe asking them to differentiate between the model/view/controller. This is too direct from the notes.

README.md Outdated
- The `show` view should include the complete information about the task: name, description, completion date.
- All HTML in views should have semantic relevance.
- Add some basic styles to your application.

Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick Either take off all the periods or put them in for all.

@droberts-sea droberts-sea merged commit 3d83384 into master Mar 20, 2017
@droberts-sea droberts-sea deleted the kab/feedback branch March 20, 2017 04:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CheezItMan CheezItMan CheezItMan approved these changes

@PilgrimMemoirs PilgrimMemoirs Awaiting requested review from PilgrimMemoirs

@droberts-sea droberts-sea Awaiting requested review from droberts-sea

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kariabancroft @CheezItMan @droberts-sea