search

faker-ruby / faker

A library for generating fake data such as names, addresses, and phone numbers.
MIT License
11.2k stars 3.18k forks source link

Namespacing for clarity #1318

Closed rpbaptist closed 1 year ago

rpbaptist commented 6 years ago

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

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:

For lorem generators:

For core features:

This would also handle this naming:

Which could be:

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 commented 6 years ago

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

rpbaptist commented 6 years ago

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

vbrazo commented 5 years ago

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 commented 5 years ago

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

justinxreese commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

vbrazo commented 5 years ago

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

Zeragamba commented 5 years ago

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

vbrazo commented 5 years ago

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 commented 5 years ago

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:

cjmz commented 5 years ago

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

Beyond the names proposes by @boardfish

boardfish commented 5 years ago

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

vbrazo commented 5 years ago

We've created a few namespaces and it's time for feedback.

Please check our unreleased_README.md out and let us know how it looks: cleaner or worse?

boardfish commented 5 years ago

Faker::Esport is a pretty glaring omission from the Games namespace, as are the following from these namespaces:

There's probably a few more in there. I also noticed the link to the Avatar documentation is broken in the README.

All that aside, I'm really happy to see that the namespacing is getting cleaned up in this gem, and I'm happy to help with some more if I get the time.

vbrazo commented 5 years ago

I've just fixed the avatar link. Thanks for reporting 👍

vbrazo commented 5 years ago

I'd also add this one:

vbrazo commented 5 years ago

@boardfish done

richardbulger commented 5 years ago

I have just added #1493 for The Culture series books by Ian M. Banks, and was surprised to not see a Books or Literature namespace. Dune and Lovecraft are a couple that should also be in there. Literature would be broader, potentially encompassing things like GreekPhilosophers and DcComics I would also argue that Hobbit, HitchhikersGuideToTheGalaxy, HarryPotter should be in there rather than in Movies (can they span multiple namespaces?)

vbrazo commented 5 years ago

Would you like to work on the Faker::Book:: namespace?

richardbulger commented 5 years ago

@vbrazo Sure, I'll give it a go!

richardbulger commented 5 years ago

How does everyone feel with generators in multiple namespaces? eg: Faker::Books::HarryPoter Faker::Movies::HarryPoter

stympy commented 5 years ago

I do not like the idea of having generators in multiple namespaces.

stympy commented 5 years ago

That said, I can see that there might be cases where it makes sense to have different data for a movie vs. a book, so I’m open to considering the question on a case-by-case basis, but I don’t want to default to having the same code/data in two namespaces just in case someone would look in one place but not the other. :)

justinxreese commented 5 years ago

Just noticed the RFC on the new README here https://github.com/stympy/faker/issues/1318?_pjax=%23js-repo-pjax-container#issuecomment-437877899

It's certainly better but still very overwhelming. I personally think the default name space should be reserved for some very basic things that we're all going to use day to day when coding. Then we can reach down into the modules when we want to have some fun. I definitely like that fun spirit Faker has but only use it after I've gotten the basics in place.

It'd be hard to figure out the 10 or so that deserve to be in the default name space, but the organization might even encourage people to play and add more classes in the modules.

I noticed this time around that there appears to be quite a few classes in the default namespace that only have one method for finding quotes (Yoda, FamousLastWords, Matz, MostInterestingManInTheWorld, Shakespeare, Robin). I could make a PR for a Faker::Quotes module if I got the go-ahead from a maintainer that it would be accepted. Mention me if that's a good idea.

Zeragamba commented 5 years ago

A Quotes module sounds like a great idea, and it would move a fair amount of the clutter out of the default namespace. Though there are ones like Yoda that would fit better if they were merged into a larger module (ie. StarWars)

justinxreese commented 5 years ago

I did think about that and agree about that making sense in either place. My best solution was making a class in Quotes that calls everything from StarWars::Yoda.

module Quotes
  class Yoda
    def quote
      StarWars::Yoda.quote
    end
  end
end

That way Yoda maintains the right to define his quotes and StarWars::Yoda can do more than quotes (random Jedi move?). Might be too duplicitous.

I like writing up samples for this repo 😆

Zeragamba commented 5 years ago

I like the feel of having things in multiple expected places. i.e. if there is a quotes module, I would expect Yoda quotes to be in there as well as the StarWars module.

vbrazo commented 5 years ago

@justinxreese last week metrics

screen shot 2019-01-01 at 10 31 41 am

justinxreese commented 5 years ago

Oh neat. So I guess it's not that hard to figure out the defaults 😁

vbrazo commented 5 years ago

@justinxreese I just opened a new PR with the suggested Quotes namespace. Let me know if we should add more faker objects to this namespace. I couldn't identify others.

boardfish commented 5 years ago

@stympy I agree that having generators in multiple namespaces adds clutter, but if implementation was similar to @justinxreese's sample and we could clearly document 'child' modules, perhaps it'd be beneficial to users of the project.

Zeragamba commented 5 years ago

It would be a big help for modules like HarryPotter

stympy commented 5 years ago

@boardfish @justinxreese Agreed. I'm on board with that approach.

vbrazo commented 5 years ago

@justinxreese last week metrics

screen shot 2019-01-17 at 10 51 10 am

justinxreese commented 5 years ago

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 commented 5 years ago

@justinxreese last week metrics

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

vbrazo commented 5 years ago

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

justinxreese commented 5 years ago

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

cjmz commented 5 years ago

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. :smile:

vbrazo commented 5 years ago

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

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

Zeragamba commented 5 years ago

WorldCup for which sport? :P

justinxreese commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

richardbulger commented 5 years ago

"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 commented 5 years ago

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

petewalker commented 5 years ago

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

stympy commented 5 years ago

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