-
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 YARD docs for Faker::Music{,::Opera} #1873
Conversation
# | ||
# @return [Array<String>] | ||
# | ||
# @faker.version 1.6.4 | ||
def keys | ||
%w[C D E F G A B] |
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.
I know it's not related to your Pull Request, I'm just commenting so we can discuss it :)
Maybe this should become an array inside the localization file or perhaps a constant? (I'd prefer the latter since it probably won't ever change?)
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.
Seems reasonable to me, I'll push another commit to this PR
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.
Oops, seems it's already been merged 🤷♂ @lucasqueiroz want me to open another?
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.
Yes, you can open a new PR please. Thanks for contributing @jas14
* Add Faker::Music YARD docs * Add Faker::MUsic::Opera YARD docs
* Add Faker::Music YARD docs * Add Faker::MUsic::Opera YARD docs
Issue#
#1762
Description:
Adds YARD docs for
Faker::Music
andFaker::Music::Opera
.