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

Namespacing for clarity #1318

Closed
rpbaptist opened this issue Jul 26, 2018 · 58 comments
Closed

Namespacing for clarity #1318

rpbaptist opened this issue Jul 26, 2018 · 58 comments

Comments

@rpbaptist
Copy link
Contributor

rpbaptist commented Jul 26, 2018

I like Faker because it's useful. I also like generating quotes over lorem ipsum. What detracts from the usefulnes of Faker is this:

  • Faker::HarryPotter
  • Faker::HeyArnold
  • Faker::Hipster
  • Faker::HitchhikersGuideToTheGalaxy
  • Faker::Hobbit
  • Faker::HowIMetYourMother
  • Faker::IDNumber
  • Faker::Internet
  • Faker::Invoice
  • Faker::Job
  • Faker::Kpop
  • Faker::LeagueOfLegends
  • Faker::Lebowski
  • Faker::LordOfTheRings

Out of this huge list, what I find useful is IDNumber and Internet. When I want to use this list as a reference there's a big list of noise to scroll through.

My suggestion is to namespace these, as such:

  • Faker::PopCulture::HarryPotter
  • Faker::PopCulture::HeyArnold
  • Faker::PopCulture::HitchhikersGuideToTheGalaxy
  • Faker::PopCulture::Hobbit
  • Faker::PopCulture::HowIMetYourMother
  • Faker::PopCulture::Invoice
  • Faker::PopCulture::Kpop
  • Faker::PopCulture::LeagueOfLegends
  • Faker::PopCulture::Lebowski
  • Faker::PopCulture::LordOfTheRings

For lorem generators:

  • Faker::Lorem::Hipster

For core features:

  • Faker::IDNumber
  • Faker::Internet
  • Faker::Job

This would also handle this naming:

  • Faker::Lorem
  • Faker::LoremFlickr
  • Faker::LoremPixel

Which could be:

  • Faker::Lorem::Ipsum
  • Faker::Lorem::Flickr
  • Faker::Lorem::Pixel

I am happy to make a PR for this, but I wanted to get some consensus on this before I do. I realize this is a breaking change, but this could of course be solved through deprecation.

@stympy
Copy link
Contributor

stympy commented Jul 26, 2018

I like your suggestion, though coming up with a taxonomy may be a challenge. :)

@rpbaptist
Copy link
Contributor Author

Yes, I settled for PopCulture here, but I readily admit it's probably not the best fit. Faker::References? That's also not perfect. 🤷‍♂️

@vbrazo vbrazo changed the title Suggestion: Namespacing for clarity Namespacing for clarity Jul 26, 2018
@vbrazo
Copy link
Member

vbrazo commented Aug 13, 2018

As the modules and methods grow, we'll definitely need to create taxonomy. I have to agree with @stympy the challenge would be defining a great taxonomy. Once we decide it, I think we could separate the changes in different releases, otherwise the number of deprecated warnings will be pretty disturbing.

@vbrazo
Copy link
Member

vbrazo commented Aug 13, 2018

This group Faker::Lorem::Ipsum, Faker::Lorem::Flickr, Faker::Lorem::Pixel looks good and could be a good starting point.

@justinxreese
Copy link

justinxreese commented Aug 18, 2018

I came to the issues to see if anyone had brought this up and @rpbaptist came up with the same scheme I was going to suggest. I think a PopCulture namespace is a priority, the list is a bit overwhelming as-is and it's primarily because of these. @rpbaptist and I independently coming to the same conclusion speaks to it being a pretty good start to the taxonomy.

I thought Faker::Community would be a good module for the user groups in my tests, but it turned out to be the TV show.
nailed_it_troy_abed

@vbrazo
Copy link
Member

vbrazo commented Aug 28, 2018

I created the first one: Faker::Lorem::Ipsum. Let me know your thoughts on that.

Feel free to work on Faker::Lorem::Flickr and Faker::Lorem::Pixel.

@vbrazo
Copy link
Member

vbrazo commented Sep 27, 2018

I've also added Faker::Lorem::Flickr and Faker::Lorem::Pixel to the same PR.

@vbrazo
Copy link
Member

vbrazo commented Sep 27, 2018

I've just added Faker::Lorem::Hipster to the PR as well. We should be ok with Lorem namespacing.

@Zeragamba
Copy link
Contributor

Here's an attempt at trying to namespace the current list of modules:
faker namespaces.txt

@vbrazo
Copy link
Member

vbrazo commented Sep 29, 2018

We also have Faker::Bitcoin, Faker::Ethereum and Faker::Tezos. They generate addresses, contracts and etc. They could be grouped in a module too.

Perhaps Faker::Blockchain::Bitcoin, Faker::Blockchain::Ethereum and Faker::Blockchain::Tezos?

@boardfish
Copy link
Contributor

boardfish commented Oct 18, 2018

I support a lot of what @SpyMaster356 is going for – much of it makes sense. I attempted to right this myself in this PR, but I didn't check there was a PR open for it first of all. In the PR, @vbrazo suggests to make lots of smaller PRs for particular namespaces, and I agree that that's probably the best way to attack this to make things easier to review.

The namespaces we want probably depends on how many levels deep we want to take things. For the sake of simplicity, I'd probably recommend just one level, which contains:

  • Books
  • Movies
  • TV
  • Comics (questionable)
    I'm guessing Marvel will get a Faker eventually, but there's also room for others like Beano. Still, I'm not sure a) if Comics is a big enough category and b) where things would go if not for comics
  • Memetic (questionable)
    Fakers like Robin, ChuckNorris and MostInterestingManInTheWorld would fit under this category, but equally, they're all part of their own media. Yoda is a good example of something that should really be within the StarWars Faker.
  • Music
    This was one thing I thought @SpyMaster356's recommendation could do better, but it's also hard not to justify making a second tier within this one containing Artist and Genre.
  • Games
  • Japanese Media
    Best to generalise Anime or Manga potential modules to this as most major manga get anime adaptations.

@cjmz
Copy link

cjmz commented Oct 18, 2018

I've got a namespaces suggestions, I've included those items that @vbrazo propose:

  • Lorem:
    Faker::Lorem::Lorem
    Faker::Lorem::LoremFlickr
    Faker::Lorem::LoremPixel

  • Personal
    Faker::Personal::Address
    Faker::Personal::FunnyName
    Faker::Personal::Name
    Faker::Personal::Nation
    Faker::Personal::PhoneNumber

  • Type
    Faker::Type::Boolean
    Faker::Type::Number
    Faker::Type::String
    Faker::Type::Types

  • Time
    Faker::Time::Date
    Faker::Time::Time

  • Economic
    Faker::Economic::Bank
    Faker::Economic::Bitcoin
    Faker::Economic::Crypto
    Faker::Economic::CryptoCoin
    Faker::Economic::Currency

Beyond the names proposes by @boardfish

@boardfish
Copy link
Contributor

I'm currently working on a PR to move DragonBall, OnePiece and SwordArtOnline into a Faker::JapaneseMedia namespace.

@justinxreese
Copy link

Looks like it stays pretty consistent, @vbrazo. Very much biographical info for people or companies. I'm assuming internet is mostly used for the email address. Often when I use Faker, I am just throwing stuff into a struct to make a thing that looks like a person.

Not sure what that means when it comes to the default namespace. Probably just that all of these ones should stay put?

@vbrazo
Copy link
Member

vbrazo commented Jan 27, 2019

@justinxreese last week metrics

screen shot 2019-01-27 at 2 29 44 pm

@vbrazo
Copy link
Member

vbrazo commented Jan 27, 2019

@justinxreese the impact of deprecating these popular faker object would be huge, so I'd prefer to keep them in the default namespace.

@justinxreese
Copy link

@vbrazo makes sense. The pain of it would outweigh any benefit.

@cjmz
Copy link

cjmz commented Jan 30, 2019

I opened a PR to add a namespace TvShows to SuperHero. This my first opensource project pull request in my life, so please, check it two times. 😄

@vbrazo
Copy link
Member

vbrazo commented Feb 14, 2019

I just moved Faker::Football to Faker::Sports::Football.

We have Faker::WorldCup. What do you guys think about Faker::Sports::WorldCup?

@Zeragamba
Copy link
Contributor

WorldCup for which sport? :P

@justinxreese
Copy link

I just checked out the new README and it's really much less intimidating now. Still some work to do but at a place where others will totally feel encouraged to contribute to the cleanliness. Great work.

I'm for Faker::Sports::WorldCup and not worried about the "which sport" problem because anyone wondering that will be already 90% of the way to their answer

@Zeragamba
Copy link
Contributor

I think we should also make the distinction between American and European Football, as they are completely different sports.

Faker::Sports::EuroFootball and Faker::Sports::UsaFootball?

@justinxreese
Copy link

As an American football fan, I won't be offended if Europeans call dibs on "football" and it's Sports::Football and Sports::AmericanFootball 😂

@Zeragamba
Copy link
Contributor

we could do Faker::Sports::EuroFootball (aliased to ::football), and Faker::Sports::AmericanFootball or Faker::Sports::UsaFootball

@richardbulger
Copy link
Contributor

"Soccer" is globally accepted as European football (even if we Brits scoff at it a bit).
Faker::Sports::AmericanFootball and Faker::Sports::Soccer seems tidier.

@stympy
Copy link
Contributor

stympy commented Feb 14, 2019

Being an American, my vote is for Sports::Football and Sports::Soccer :)

@petewalker
Copy link

Hi all, whilst I applaud the namespacing effort, the method you're using to deprecate is breaking backwards-compatibility. See #1576

@stympy
Copy link
Contributor

stympy commented Mar 18, 2019

Having had some time living with namespaces, I’m liking them less and less. I’m considering reverting the whole change.

@Zeragamba
Copy link
Contributor

I think we should keep the namespacing as we have a huge number of generators (~160 !), and there are some cases where it's not exactly clear whats in each generator (as metioned by @justinxreese near the beginning of this issue).

We've already been sending out deprecation notices since the release of v1.9.2 (2019-02-11). I would expect that there would be a lot of annoyance from users if we reversed back to no namespaces.

@stympy
Copy link
Contributor

stympy commented Mar 18, 2019

That sounds like a +1 for releasing v2, which will have plenty of backwards incompatibility, sooner rather than later. :)

@vbrazo
Copy link
Member

vbrazo commented Mar 18, 2019

Namespaces are okay as long as we try to have good generators. I'd prefer to start thinking about the generators that aren't useful.

Contributors should explicitly tell us why they're adding a new generator by giving a use case in the PR description. I think we should add a GitHub template for the PRs and ask for that.

@Zeragamba
Copy link
Contributor

Speaking of good generators, there are a few bad ones that have only one method in them (eg, Faker::Creature::Animal, Faker::Artist, Faker::TvShows::MichaelScott) We could merge some of these into other generators.

@boardfish
Copy link
Contributor

@SpyMaster356 Perhaps single-method fakers could go back into the top-level generator, so we'd get Faker::Creature.animal and Faker::TvShows.michael_scott_quote for example? I can't really say this with full conviction as I get the feeling it's got caveats, but right now I can't think what those might be. Feel free to propose something else.

@vbrazo vbrazo unpinned this issue Jul 9, 2019
@PatrickLerner
Copy link

I would advocate to delete all the pop culture stuff from the main gem (who needs most of it really? and it just adds a ton of PR where everybody wants to get their list of random things into this gem) and if you really want lord of the rings character names, it can be a separate gem (lotr-faker). But I guess I am in the minority here as people seem to enjoy this.

@boardfish
Copy link
Contributor

boardfish commented Dec 15, 2020

@PatrickLerner I imagine this was part of the case for #1539. A project of mine, RemoteRecord will most likely do something like this. I'm choosing to take that approach for a few reasons, but they're also reasons that support Faker retaining its current structure as I'll explain.

It might help to check it out for a bit of context, but I'm hoping my explanations here are framed in a way that can be understood without it. What's important here is that RemoteRecord plugs into any API you need, and that introduces the following differences:

  • It's the responsibility of the community, not the base project, to publicize and maintain each individual module. In RemoteRecord's case, this is better because it's often in the interest of companies to have their own developers maintain their RemoteRecord extensions. The same can't really be said for Faker, in which all we're dealing with is text. As long as the writers know the subject matter and are literate, anyone can manage Faker's content library.
  • Faker doesn't particularly need to be extended manually by its users since it has such a wide scope already. RemoteRecord (at least right now) will almost always want to be extended manually by writing code in app/lib/remote_record. The next stage from that is having a set of gems like remote_record-github and remote_record-spotify that folks can install as they need. Faker comes with an extensive base scope that makes this unnecessary - changing to that sort of structure, more importantly, would put more friction in the way of using Faker.

@stefannibrasil
Copy link
Contributor

Hey, folks. In an effort to lighten our load as maintainers and be able to serve you better in the future, the faker-ruby team is working on cleaning out the cobwebs in this repo by pruning the backlog. As there are few of us, there are a lot of items that will simply never earn our attention in a reasonable time frame, and rather than giving you an empty promise, we think it makes more sense to focus on more recent issues. That means, unfortunately, that we must close this issue.

Don't take this the wrong way: our aim is not to diminish the effort people have made or dismiss problems that have been raised. If you feel that we should reopen this issue, then please let us know so that we can reprioritize it. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests