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.

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

add request cycle and more detailed explanation of mvc #636

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

CZagrobelny
Copy link

This PR Contains:

  1. Minor syntax/grammar changes
  2. More in depth explanation of migrations.
  3. Swap written description of files created by scaffolding with written/visual/exercise-based explanation.
  4. Remove nesting for the Rails Architecture page, expanded explanation of MVC and it's role in the request cycle with diagram to demonstrate.

We workshopped these changes over the course of a year with the NYC Railsbridge workshops. We found that a deeper understanding of MVC, Request Cycle, and Databases was very helpful throughout the Intro to Rails tutorial. We also saw real value in substituting visuals and exercises with text where possible.

@CZagrobelny
Copy link
Author

@javierjulio @maxjacobson please take a look!

Copy link

@javierjulio javierjulio left a comment

Choose a reason for hiding this comment

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

@CZagrobelny this looks great thanks! I only have two comments. One is just to use rails over rake for consistency and the other is just a little more content on partials.

end
```

`rake db:migrate` is a task provided by the Rails framework. It uses the migration file we just created (`db/migrate/201xxxxxxxxxxx_create_topics.rb`) to change the database.

Choose a reason for hiding this comment

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

Should be rails db:migrate

Copy link
Author

Choose a reason for hiding this comment

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

@javierjulio nice catch! Thought I got em all....

message "You may have noticed that the page for new topics and the page to edit topics looked similar. That's because they both use the
code from this file to show a form. This file is called a
partial since it only contains code for part of a page. Partials
always have filenames starting with an underscore character."
Copy link

@javierjulio javierjulio Feb 19, 2018

Choose a reason for hiding this comment

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

This is fine but it felt like a raw definition without giving much clear reason as to why we'd use partials. We can expand just a little bit. I only changed the last 2 sentences. The additions are marked in bold:

This file is called a partial since it only contains code for rendering part of a page. Partials are easy to spot as the filename always starts with an underscore character. They are great for organizing page rendering code into smaller, manageable parts.

I like this since it highlights two important parts: convention and maintenance.

Copy link
Author

Choose a reason for hiding this comment

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

👍 changing!

@CZagrobelny
Copy link
Author

@rachelfenn would love to get a review on this when you have a chance! Please let me know if you have any questions! Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants