Introduce a unified Tolkien Legendarium #2152
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 12 commits
October 5, 2020 15:13
Also, do not wrap lines at 81 chars (Pysch default)
Zeragamba
suggested changes
Oct 10, 2020
| # frozen_string_literal: true | ||
|
|
||
| module Faker | ||
| class TolkienLegendarium < Base |
There was a problem hiding this comment.
This should be moved to a name space as we'd like to reduce the number of generators in the default namespace.
Perhaps this could be moved to Faker::Fantasy::Tolkien?
There was a problem hiding this comment.
I love it. It shall be done.
There was a problem hiding this comment.
@Zeragamba Great call on the namespace. Much cleaner. I'm tempted to do some additional cleanup. If we wanted to move additional "Fantasy" generators to this new namespace, would we follow a similar pattern that I used here to allow for a deprecation/sunsetting window for the old namespaces?
There was a problem hiding this comment.
we can move some of the other fantasy stuff in other PRs. For now, this PR's quest is done
Zeragamba
approved these changes
Oct 10, 2020
droznyk
pushed a commit
to droznyk/faker
that referenced
this pull request
Oct 23, 2020
* Combine lotr and hobbit into tolkien .yml * Add tolkien characters * Restore hobbit and lotr movie yamls * Fix generator to use new legendarium class * Combine hobbit and lotr yml into legendarium yml * Add indentation to yaml array list items Also, do not wrap lines at 81 chars (Pysch default) * Point tolkien movies to default lengendarium yml * Remove silly comment * Use consistent quotations. Unbreak lines. * Alias 'movie' classes. Point to new yaml file name * Add tests * Fix formatting and documentation * Re-namespace to fantasy/tolkien
droznyk
added a commit
to droznyk/faker
that referenced
this pull request
Oct 23, 2020
This reverts commit ac813b8.
droznyk
added a commit
to droznyk/faker
that referenced
this pull request
Oct 23, 2020
This reverts commit ac813b8.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue#
No-StoryDescription:
The existing generators for "The Lord of the Rings" and "The Hobbit" are namespaced under the guise of
Movies, but a quick glance at the quotes sections reveals that at least some of the content is from the books. Instead of making a separate instance of book and movie content, I have combined them into a single "Legendarium". This is in keeping with the pattern present in the unifiedCosmereclass.In order to prevent breaking existing implementations, the "Movie" generators will remain in place, but their
.ymlentries have been made as nested objects in the new unified legendarium data.Additionally, I have introduced, what I believe to be, a complete list of ALL Tolkien characters, races, poems, and locations from the legendarium. Most data was scraped from the Tolkien Gateway Wiki. I have deduplicated, removed parentheticals, and normalized all the entries.
BONUS
I made a change to
reformat_yaml.raketo make its formatting match the examples inCONTRIBUTING.md.