-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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 faker for hiphop artist #1923
Conversation
lib/faker/music/hiphop.rb
Outdated
# @example | ||
# Faker::Music::Hiphop.group #=> "OVO" | ||
# | ||
# @faker.version 1.9.2 |
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.
should be @faker.version next
so that we can just find and replace when this gets merged
lib/faker/music/hiphop.rb
Outdated
# @example | ||
# Faker::Music::Hiphop.artist #=> "Lil Wayne" | ||
# |
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.
incomplete DocBlock
lib/faker/music/hiphop.rb
Outdated
# @return [String] | ||
# | ||
# @example | ||
# Faker::Music::Hiphop.group #=> "OVO" |
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.
example shows the wrong method
@Zeragamba if you got a chance |
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.
Minor changes needed, and this is good to go
lib/faker/music/hiphop.rb
Outdated
# Faker::Music::Hiphop.groups #=> "OVO" | ||
# | ||
# @faker.version next | ||
|
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.
Extra space here that can be removed
lib/locales/en/hiphop.yml
Outdated
@@ -0,0 +1,127 @@ | |||
en: | |||
faker: | |||
hiphop: |
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.
Can we shift this down to en.faker.music.hiphop?
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.
@Zeragamba got all the comments knocked out.
lib/faker/music/hiphop.rb
Outdated
end | ||
|
||
## | ||
# Produces the name of a Hip Hop Group |
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.
Copy and paste issue :P
lib/locales/en/music.yml
Outdated
@@ -30,3 +30,14 @@ en: | |||
"Flashdance: Original Soundtrack from the Motion Picture", "Whitney", "Confessions", "X&Y", "High School Musical", "High School Musical 2", "Viva la Vida or Death and All His Friends", | |||
"I Dreamed a Dream", "Recovery", "Midnight Memories", "Frozen", "Lemonade", "Brand New Eyes", "All We Know Is Falling", "Riot!", "Songs About Jane", "Hands All Over"] | |||
genres: ["Rock", "Pop", "Electronic", "Folk", "World", "Country", "Jazz", "Funk", "Soul", "Hip Hop", "Classical", "Latin", "Reggae", "Stage And Screen", "Blues", "Non Music", "Rap"] | |||
hiphop: |
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.
We would prefer single item per line for yaml arrays so that it's easier to see when something is added or removed
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.
can you past an example so I do it right @Zeragamba
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.
Sorry for the radio silence. Check CONTRIBUTING.md
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.
should be fixed now @Zeragamba
Thanks! |
* add faker for hiphop artist * update faker docs * update comments based on feedback * clean up * reformat
This reverts commit 01494d4.
This reverts commit 01494d4.
This PR adds names of Hip Hop Artist & Groups to Faker Gem For Users to use