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

Changes to Ruby Language section in Intro to Rails Tutorial #635

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

Conversation

CZagrobelny
Copy link

@CZagrobelny CZagrobelny commented Feb 12, 2018

This PR changes the 'Ruby Language' section of the 'Intro to Rails' tutorial. These changes came out of the NYC Chapter iterating on the Intro to Rails curriculum over the course of a year. These changes have been working great in our workshops and we would love to get them merged upstream.

  1. Adds a 'hash' section.
    Reason for Change: We found hashes to be a critical underlying concept for understanding Rails code where a hash is passed in as an argument and understanding the format of Active Record objects.

  2. Additional explanation around the concept of 'chaining' methods.
    Reason for Change: We found that this was an area of confusion for students.

  3. Replaces wrap up question with an optional problem to solve in small groups.
    Reason for Change: We found students learned the most when challenged to work together with a partner to solve a problem.

  4. Spacing changes to enhance readability for developers editing these docs, no impact when rendered.

@CZagrobelny
Copy link
Author

@javierjulio @maxjacobson please take a look!

@javierjulio
Copy link

These changes look good so far. I’m not on my Mac though but I’ll take a look again once I am. One suggestion is we should use squiggly heredoc <<~ instead as that way you can format how you like in the file but when rendered the content is right on the left edge.

@CZagrobelny
Copy link
Author

@javierjulio I'm not totally following your suggestion re: formatting. If it's quick, when you take a look on your mac, could you push a commit with the change you had in mind? Thanks for looking at it!!!

@javierjulio
Copy link

Sure, no problem. Sorry, I’m arriving at JFK ato pick up my mother. I’ll follow up tomorrow. Thanks!

@maxjacobson
Copy link

I think Javier is recommending using this Ruby 2.3 feature: https://infinum.co/the-capsized-eight/multiline-strings-ruby-2-3-0-the-squiggly-heredoc

I don't think it's necessary here, because the helper method already uses strip_heredoc which accomplishes the same thing:

pre commands.strip_heredoc

This PR looks great to me -- these tweaks have all helped us out in New York, and getting them merged upstream will make it easier for us to deprecate our fork.

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 I spent more time now going through this and it looks good. I really like the hash changes which are all very descriptive. 👍🏻 Thanks!

RUBY

message "And **chain** methods together:"
message "And you can call multiple methods in a row:"

Choose a reason for hiding this comment

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

Not a big deal but I think its helpful to have some mention of "chaining" here well assuming there isn't already on this page because "method chaining" is such a common term. I guess its obvious but figure its helpful to mention something like "also referred to as method chaining"?

Copy link
Author

Choose a reason for hiding this comment

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

Yah, I thought about this a bit. For me, it's a level of detail that for beginners, in particular, will likely be forgotten since there is so much information coming at them, which is why I decided to keep this change from the Boston docs.

@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.

None yet

3 participants