-
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::TvShow namespace #1431
Conversation
since we will have namespaces now, we can sort the tests into folders to make it easier to navigate
Also leave deprecation warning in both the top level and the movies namespace
and add missing unit tests
and add missing documentation
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 think you'll need to merge/rebase your branch with master and move your docs to the new folders. You should update the unreleased_CONTENT.md
instead of README.md
.
sorry for the delay in getting that stuff merged |
any additional changes needed for this? |
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.
Most of the changes are pretty good and I think the namespace idea was great. I left a few comments as suggestions. Let me know your thoughts.
def character | ||
fetch('aqua_teen_hunger_force.character') | ||
Faker::TvShows::AquaTeenHungerForce.character | ||
end |
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.
👍
end | ||
|
||
deprecate :character, 'Faker::TvShows::AquaTeenHungerForce.character', 2018, 10 |
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 think the deprecate dates should be 2018, 12
@@ -2,7 +2,7 @@ | |||
|
|||
require_relative 'test_helper' | |||
|
|||
class TestFakerRickAndMorty < Test::Unit::TestCase | |||
class TestDeprecateRickAndMorty < Test::Unit::TestCase |
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.
When we call a deprecate message, we get a standard message. Could we test if we get this behaviour?
end | ||
|
||
deprecate :character, 'Faker::TvShows::SouthPark.character', 2018, 10 | ||
deprecate :quote, 'Faker::TvShows::SouthPark.quote', 2018, 10 |
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 need to review this class because I've changed its namespace recently and we haven't released my PR yet, so I think the deprecate should be different #1403
* add TvShows namespace * move BojackHorseman into TvShows namespace * update documentation for BojackHorseman * reorganize the new test since we will have namespaces now, we can sort the tests into folders to make it easier to navigate * move AquaTeenHungerForce to TvShows namespace * remove Bojack from the ignored subclasses * move BreakingBad to TvShows * move Buffy to TvShows * move DrWho to TvShows * update Readme * move Community to TvShows * make deprecated methods wrappers around new ones * add flexible tag to new classes * move DumbAndDumber to TvShows * update Faker::TvShows::BreakingBad to use class << self style * fix filename * Move FamilyGuy to TvShows * move VentureBros to TvShows * move TwinPeaks to TvShows * move Friends to TvShows * move SouthPark to TvShows Also leave deprecation warning in both the top level and the movies namespace * move GameOfThrones to TvShows * move TheITCrowd to TvShows * move HeyArnold to TvShows * move HowIMetYourMother to TvShows * move NewGirl to TvShows * move ParksAndRec to TvShows * move RickAndMorty to TvShows * move Seinfeld to TvShows * move SiliconValley to TvShows * move Simpsons to TvShows * move StarTrek to TvShows * move StrangerThings to TvShows and fix typos * move TheFreshPrinceOfBelAir to TvShows * move TheThickOfIt to TvShows and add missing unit tests * move Stargate to TvShows and add missing documentation * add StarTrek back into the readme * add flexible tag to FamilyGuy * fix copy-paste error with TheITCrowd docs * please the Rubocop * remove windows platform from .lock file * update unreleased README * Restore README * Add a few comments - faker.rb * Update CHANGELOG.md
I think I got all the TV shows in the current list into the new namespace.
Please double check the deprecation notices, as there was a lot of copy-paste-next file-copy-paste-etc..