Skip to content

Changed specie to species and species to species_list in StarWars. Ma…#706

Closed
yakryder wants to merge 6 commits intofaker-ruby:masterfrom
yakryder:starwars_species
Closed

Changed specie to species and species to species_list in StarWars. Ma…#706
yakryder wants to merge 6 commits intofaker-ruby:masterfrom
yakryder:starwars_species

Conversation

@yakryder
Copy link
Copy Markdown
Contributor

…de corresponding changes in test file.

Specie is not a thing

@stympy stympy added this to the 2.0 milestone Dec 19, 2016
@stympy
Copy link
Copy Markdown
Contributor

stympy commented Dec 19, 2016

Added to 2.0 milestone since this change breaks backwards compatibility.

@yakryder
Copy link
Copy Markdown
Contributor Author

Yeah I should have thought of a deprecation path. Would be easy enough to implement

@stympy
Copy link
Copy Markdown
Contributor

stympy commented Dec 25, 2016

Yeah, I'd be happy to merge this sooner if it had a deprecation path.

@yakryder
Copy link
Copy Markdown
Contributor Author

Took a first stab at a deprecation path. Two negatives are noteworthy:

  1. I didn't add a test for the deprecation warning for StarWars.specie-- assert_output did not appear to be available and it seemed better to have a conversation before trying to work around that.

  2. Right now, the StarWars.specie deprecation warning clutters up STDOUT whenever tests are run. It's not a huge deal, but it is certainly a bothersome addition I would prefer to see resolved.

@yakryder
Copy link
Copy Markdown
Contributor Author

Fixed issue 2 above.

@vbrazo vbrazo force-pushed the master branch 5 times, most recently from a359def to a5d7731 Compare May 22, 2018 21:15
@vbrazo vbrazo mentioned this pull request Sep 4, 2018
@vbrazo
Copy link
Copy Markdown
Member

vbrazo commented Nov 11, 2018

We're moving a few objects to new namespaces and I believe Faker::StarWars could be moved to Faker::Movies::StarsWars. @Sherspock would you be up to wrap this object in the Faker::Movies namespace? We'll have to deprecate its methods anyway.

Here is a similar example: #1424

@vbrazo
Copy link
Copy Markdown
Member

vbrazo commented Nov 11, 2018

I think this issue was already solved here and here.

@vbrazo vbrazo closed this Nov 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants