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

Cleanup books. Add Fantasy::Tolkien to README. #2154

Merged
merged 4 commits into from Mar 20, 2021

Conversation

mathisto
Copy link
Contributor

Issue#

No-Story
Cleanup

Description:

This PR:

  1. Reformats the YAML for the books generators.
  2. Adds the Fantasy::Tolkien generator to the README.
  3. And adds docs entry to Fantasy::Tolkien

@@ -28,6 +28,7 @@ development.
- [Default](#default)
- [Blockchain](#blockchain)
- [Books](#books)
- [Fantasy](#fantasy)
Copy link
Member

@vbrazo vbrazo Oct 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does Fantasy do and why do we need a new namespace?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Came in with the new Faker::Fantasy::Tolkien. Since LotR and the Hobbit are equally popular as both movies and books, it didn't really make sense for that stuff to be in either. Since Tolkien is often attributed to being the grandfather of the modern fantasy world, I thought it was fitting to create a new namespace for it there.

We can alias some of the other fantasy stuff into Fantasy later.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fantasy was the new namespace I was asked to add in PR #2152 to consolidate and normalize the existing Tolkien generators (Movies::Hobbit and Movies::LordOfTheRings). The quotes section in these 'movie' generators betray their origin as books.

Copy link
Contributor

@Zeragamba Zeragamba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. I'll wait for @vbrazo to approve as well.

@mathisto
Copy link
Contributor Author

If it's not too much trouble, I'd like to request this PR receive the hacktoberfest-accepted tag. 🙇

@Zeragamba Zeragamba mentioned this pull request Oct 13, 2020
@Zeragamba Zeragamba requested a review from vbrazo October 13, 2020 15:05
@vbrazo vbrazo merged commit 64baa40 into faker-ruby:master Mar 20, 2021
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

3 participants