-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Comments
I like your suggestion, though coming up with a taxonomy may be a challenge. :) |
Yes, I settled for |
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. |
This group |
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 I thought |
I created the first one: Feel free to work on |
I've also added |
I've just added |
Here's an attempt at trying to namespace the current list of modules: |
We also have Perhaps |
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:
|
I've got a namespaces suggestions, I've included those items that @vbrazo propose:
Beyond the names proposes by @boardfish |
I'm currently working on a PR to move |
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? |
@justinxreese last week metrics |
@justinxreese the impact of deprecating these popular faker object would be huge, so I'd prefer to keep them in the default namespace. |
@vbrazo makes sense. The pain of it would outweigh any benefit. |
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. 😄 |
I just moved We have |
WorldCup for which sport? :P |
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 |
I think we should also make the distinction between American and European Football, as they are completely different sports.
|
As an American football fan, I won't be offended if Europeans call dibs on "football" and it's |
we could do |
"Soccer" is globally accepted as European football (even if we Brits scoff at it a bit). |
Being an American, my vote is for Sports::Football and Sports::Soccer :) |
Hi all, whilst I applaud the namespacing effort, the method you're using to deprecate is breaking backwards-compatibility. See #1576 |
Having had some time living with namespaces, I’m liking them less and less. I’m considering reverting the whole change. |
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. |
That sounds like a +1 for releasing v2, which will have plenty of backwards incompatibility, sooner rather than later. :) |
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. |
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. |
@SpyMaster356 Perhaps single-method fakers could go back into the top-level generator, so we'd get |
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. |
@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:
|
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! |
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
andInternet
. 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.
The text was updated successfully, but these errors were encountered: