-
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
Plays and musicals #1485
Plays and musicals #1485
Conversation
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 left a comment. You also need to update our unreleased_README.md
.
Wrapped show.rb in Faker::Music::Show, verified tests all good, updated |
Changes look good to me |
Hi, is this getting merged anytime soon? @Zeragamba seems to have blessed it and I think I complied with all the change requests. Would like to get back to using master branch rather than my fork! Thanks |
69291b2
to
5cc660c
Compare
a2a9f17
to
4384168
Compare
Hi, just checking the status of this again. After @Zeragamba signed off on the changes I didn't see any further activity assigned to me before this could be merged, but it still shows as open. |
Looks like there are conflicts that need to be resolved first, and then we can merge this in after having the PR open for... A year. Ouch. Sorry about that |
…w new test file conventions
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.
Two minor changes needed, and we're good to go :)
Co-authored-by: Stephen A. Wilson <stephen-356@hotmail.com>
yep, unreleased_README.md was deleted in master and doesn't seem to be part of the PR, and i accepted your suggested rdoc changes.
so we're good?
thx!
Armando Fox (pronouns: he, him; él)
Professor, Computer Science Division
Faculty Advisor, Digital Learning Strategy & MOOCLab
UC Berkeley Campus Equity Advisor
581 Soda Hall MC#1776, Berkeley, CA 94720-1776
+1.510.642.6820 / http://www.cs.berkeley.edu/~fox <http://www.cs.berkeley.edu/~fox>
Learn to build software free at saas-class.org
… On May 16, 2020, at 16:56, Stephen A. Wilson ***@***.***> wrote:
@Zeragamba requested changes on this pull request.
Two minor changes needed, and we're good to go :)
In unreleased_README.md <#1485 (comment)>:
> @@ -0,0 +1,365 @@
+
I think this file was deleted in master.
In lib/faker/music/show.rb <#1485 (comment)>:
> + def adult_musical
+ fetch('show.adult_musical')
+ end
+
+ def kids_musical
+ fetch('show.kids_musical')
+ end
+
+ def play
+ fetch('show.play')
+ end
⬇️ Suggested change
- def adult_musical
- fetch('show.adult_musical')
- end
-
- def kids_musical
- fetch('show.kids_musical')
- end
-
- def play
- fetch('show.play')
- end
+ ##
+ # Produces the name of a musical for an older audience
+ #
+ # @return [String]
+ #
+ # @example
+ # Faker::Alphanumeric.alpha
+ # #=> "West Side Story"
+ #
+ # @faker.version next
+ def adult_musical
+ fetch('show.adult_musical')
+ end
+
+ ##
+ # Produces the name of a musical for a younger audience
+ #
+ # @return [String]
+ #
+ # @example
+ # Faker::Alphanumeric.alpha
+ # #=> "Into the Woods JR."
+ #
+ # @faker.version next
+ def kids_musical
+ fetch('show.kids_musical')
+ end
+
+ ##
+ # Produces the name of a play
+ #
+ # @return [String]
+ #
+ # @example
+ # Faker::Alphanumeric.alpha
+ # #=> "Death of a Salesman"
+ #
+ # @faker.version next
+ def play
+ fetch('show.play')
+ end
We've been adding in Yard support over the last year. Here's the doc block needed
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub <#1485 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACCMTHXAWWB3OTE5MUUXELRR4R23ANCNFSM4GGVQG6Q>.
|
Boom |
Added code, data, tests for Broadway plays and musicals, subdivided into
play
,adult_musical
,kids_musical
. Tests & rubocop all passing and rebased to master in upstream repo.