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

Unicorn makes riders magical #2

Merged
merged 1 commit into from Feb 9, 2021
Merged

Unicorn makes riders magical #2

merged 1 commit into from Feb 9, 2021

Conversation

robertguturing
Copy link
Owner

@robertguturing robertguturing commented Feb 9, 2021

What does this Pull Request Do

  • Create the unicorn class and unicorn can make riders magical

Gif here

Type of Pull Request

  • Feature
  • Bug
  • Improvement/Refactor

Anything in particular you are requesting from reviewer?

  • Does the logic make sense for the unicorn?
  • Any other part of a standard review

PR Checklist

  • Does the code follow our teams convention?
  • Are there appropriate tests?
  • Reviewer has approved Pull Request?
  • Reviewer has merged in Pull Request?

Copy link
Collaborator

@memcmahon memcmahon left a comment

Choose a reason for hiding this comment

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

Made some notes - also, let's build some tests for these classes :)

end

def make_magic
@name = "Yer a wizard #{name}"
Copy link
Collaborator

Choose a reason for hiding this comment

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

When we make a rider magic, should we maintain their non-magic name for use in the non-magic world?

@@ -0,0 +1,23 @@
class Unicorn
attr_accessor :name
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this could be an attr_reader - is there a reason that we are exposing this?

Comment on lines +10 to +12
def exclaim_name
p name.upcase
end
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes! I love when unicorns can speak!

end

def add_rider(rider)
@riders<< rider
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's put a space here between @riders and <<

end

def make_riders_magical
riders.length.times do |i|
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's use each here

@robertguturing robertguturing merged commit 81f8078 into main Feb 9, 2021
@robertguturing robertguturing deleted the unicorn-setup branch February 9, 2021 16:56
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