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

Update South Park #2744

Merged
merged 5 commits into from
Apr 18, 2023
Merged

Update South Park #2744

merged 5 commits into from
Apr 18, 2023

Conversation

IvanReyesO7
Copy link
Contributor

Summary

  • Updated syntax for YAML arrays
  • Updated characters list
  • Added more quotes
  • Added episode names

Other Information

2087 tests, 304902 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed

@IvanReyesO7 IvanReyesO7 changed the title Updated South Park Update South Park Apr 10, 2023
Copy link
Member

@psibi psibi left a comment

Choose a reason for hiding this comment

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

LGTM, apart from minor comment.

lib/locales/en/south_park.yml Outdated Show resolved Hide resolved
lib/locales/en/south_park.yml Outdated Show resolved Hide resolved
Copy link
Contributor

@stefannibrasil stefannibrasil left a comment

Choose a reason for hiding this comment

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

Hi! Thanks for using the new YAML format for the values.

I left a comment about using words that are not inclusive. Thanks.

@@ -4,4 +4,6 @@
Faker::TvShows::SouthPark.character #=> "Mr. Garrison"

Faker::TvShows::SouthPark.quote #=> "I'm just getting a little cancer Stan"

Faker::TvShows::SouthPark.episode #=> "Make Love, Not Warcraft"
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you thought of using episode_name instead of just episode? When I read "episode", I'm not sure if it means the name, the number, the full name, i.e, S01 EP01, so defining it explicitly helps with that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Need to update the docs as well @IvanReyesO7

@@ -1,5 +1,418 @@
en:
faker:
south_park:
characters: ["Sharon Marsh", "Officer Barbrady", "Jesus", "Token Black", "Dr. Alphonse Mephesto", "Stephen Stotch", "Heidi Turner", "Jimmy Valmer", "Sheila Broflovski", "Jimbo Kern", "Ike Broflovski", "Kevin McCormick", "Father Maxi", "Grandpa Marvin Marsh", "Clyde Donovan", "Butters Stotch", "Shelly Marsh", "Kyle Broflovski", "Stuart McCormick", "Carol McCormick", "Timmy Burch", "Ned Gerblansky", "Mr. Mackey", "Satan", "Moses", "PC Principal", "Bradley Biggle", "Randy Marsh", "Kenny McCormick", "Terrance and Phillip", "Mr. Slave", "Sergeant Harrison Yates", "Lemmiwinks", "Mr. Hankey", "Wendy Testaburger", "Santa", "God", "Stan Marsh", "Towelie", "Gerald Broflovski", "Bebe Stevens", "Starvin' Marvin", "Karen McCormick", "David Rodriguez", "Eric Cartman", "Mayor McDaniels", "Tuong Lu Kim", "Tweek Tweak", "Dougie", "Craig Tucker", "Mr. Garrison", "Pip", "Liane Cartman", "Scott Malkinson", "Linda Stotch"]
quotes: ["Hippies. They're everywhere. They wanna save Earth, but all they do is smoke pot and smell bad", "They took our deers", "Kenny’s family is so poor that yesterday, they had to put their cardboard box up for a second mortgage", "Without evil there could be no good, so it must be good to be evil sometimes", "Dad, Tom Cruise won't come out of the closet!", "They took der derrs", "I'm not just sure, I'm HIV positive", "I don't make the rules Kyle, I simply think them up and write them down", "I'm just getting a little cancer Stan", "Respect my authoritaahh!!!", "Your mother was worried sick and I was here drinking beer", "Hey Panda Bear! We don't take kindly to your types around here.", "You know what they say: You can't teach a gay dog straight tricks", "They took our jobs", "Maybe we should send you to a concentration camp", "Life is short butters, & thats why you have to do whatever you want all the time", "No we haven't actually seen it Tom, we're just reporting it"]
characters:
Copy link
Contributor

@stefannibrasil stefannibrasil Apr 14, 2023

Choose a reason for hiding this comment

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

I know this hasn't been documented anywhere in the library but adding any quotes that involve sexuality, gender, and other quotes/characters/episodes that might be offensive to some people is not what Faker is intended to be.

Please, could you leave out the ones that involve body parts, sexual orientation, violence, and any other words that are not inclusive? Thank you.

Copy link
Contributor

@stefannibrasil stefannibrasil left a comment

Choose a reason for hiding this comment

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

Thanks for removing the obvious language that can be hurtful for some people. Need to update the docs and remove duplicated entries in the YAML file. Thanks!

Comment on lines +24 to +26
- Karen McCormick
- Kenny McCormick
- Kevin McCormick
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- Karen McCormick
- Kenny McCormick
- Kevin McCormick
- Karen McCormick

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@stefannibrasil Thank you for reviewing this!
Those three are different characters with the same family name.

Copy link
Contributor

Choose a reason for hiding this comment

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

That was easy to get it wrong 😆

@@ -4,4 +4,6 @@
Faker::TvShows::SouthPark.character #=> "Mr. Garrison"

Faker::TvShows::SouthPark.quote #=> "I'm just getting a little cancer Stan"

Faker::TvShows::SouthPark.episode #=> "Make Love, Not Warcraft"
Copy link
Contributor

Choose a reason for hiding this comment

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

Need to update the docs as well @IvanReyesO7

@stefannibrasil stefannibrasil merged commit 6aae8e2 into faker-ruby:main Apr 18, 2023
@IvanReyesO7 IvanReyesO7 deleted the update-south-park branch April 18, 2023 06:29
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.

3 participants