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

Adds Smashing Pumpkins to the Music module #2817

Merged
merged 1 commit into from Nov 1, 2023

Conversation

redconfetti
Copy link
Contributor

@redconfetti redconfetti commented Aug 29, 2023

Adding the Smashing Pumpkins to the Music module

Description:
New Faker::Music::SmashingPumpkins class that contains the following methods:

musician - will return the name of a band member (former or current)
album - will return the name of one of the band's albums
song - will return the name of one of their songs
lyric - will return a lyric from one of their songs

Faker::Music::SmashingPumpkins.musician #=> "Billy Corgan"
Faker::Music::SmashingPumpkins.musician #=> "James Iha"
Faker::Music::SmashingPumpkins.album #=> "Machina/The Machines of God"
Faker::Music::SmashingPumpkins.album #=> "American Gothic"
Faker::Music::SmashingPumpkins.song #=> "Fireflies"
Faker::Music::SmashingPumpkins.song #=> "Cottonwood Symphony"
Faker::Music::SmashingPumpkins.song #=> "Annie-Dog"
Faker::Music::SmashingPumpkins.song #=> "Gossamer"
Faker::Music::SmashingPumpkins.lyric #=> "We all want to hold in the everlasting gaze, enchanted in the rapture of his sentimental sway, but underneath the wheels lie the skulls of every cog, the fickle fascination of an everlasting God"
Faker::Music::SmashingPumpkins.lyric #=> "And the embers never fade, in your city by the lake. The place where you were born"
Faker::Music::SmashingPumpkins.lyric #=> "So meets the final coda of a vinyl storm, one more cherry cola to lift up her dead arms. A dream of soft focus, sunsets filters through the dim. We are losing contact as she dials it in"

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 @redconfetti please review the general guidelines for contributing to faker: https://github.com/faker-ruby/faker/blob/main/CONTRIBUTING.md#general-guidelines

I only gave a quick review and there are some words that we don't allow.

Additionally, could you half the number of songs and lyric? We don't need to add all of them, it's a lot to review and maintain. Thanks!

@redconfetti
Copy link
Contributor Author

@stefannibrasil - The guidelines state:

Don't include hurtful language that can convey exclusionary behavior, such as racism, sexism, homophobia. Be considerate and mindful of others.

I'm not sure which words you would categorize under this, but I've removed what I think might be vaguely categorized as this.

I reduced the songs to ones on primary album releases. Also reduced lyrics to ones most popular.

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 @redconfetti thank you for your patience, I was on PTO. Yes, you're right -- it's a bit ambiguous. We're still working on that, so I appreciate your understanding.

The file still has a lot of lyrics so it was a bit hard to review still. The first time, I remember finding words such as "wh****", which is ambiguous. Thanks for giving it another round of review.

I left just one remaining suggestion. I worry folks who have had situations with the phrase could have a bad experience using the library.

Once that one is removed, this is good to go. Thank you!

lib/locales/en/smashing_pumpkins.yml Outdated Show resolved Hide resolved
@stefannibrasil stefannibrasil merged commit 555d9fa into faker-ruby:main Nov 1, 2023
7 checks passed
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

2 participants