thephpleague / factory-muffin

Enables the rapid creation of objects for testing
https://factory-muffin.thephpleague.com/
MIT License
531 stars 72 forks source link

Removing framework specific keyword #390

Closed jadb closed 9 years ago

jadb commented 9 years ago

I removed "laravel" from the keywords after addressing this concern to @philsturgeon a couple days ago. I also re-ordered the list of suggestions.

GrahamCampbell commented 9 years ago

Framework-agnostic library

We are framework agnostic, but we also are geared to laravel too. I want to keep that suggest so people can find this package when searching for laravel packages.

lorenzo commented 9 years ago

Are there plans to be more agnostic like the rest of the packages in the league? It looks odd recommending a full stack framework for a testing library. Reminds me of a small package I installed a few weeks ago that was recommending me to require the full zend framework 1as well

lorenzo commented 9 years ago

Correction: I thought the packages was in suggests instead of keywords, I read more carefully again

jadb commented 9 years ago

@GrahamCampbell understood (in a way). Can I then add CakePHP to the keywords so it can also be found when looking for testing and cakephp?

Thanks for merging #391 by the way, much appreciated!

GrahamCampbell commented 9 years ago

Can I then add CakePHP to the keywords so it can also be found when looking for testing and cakephp?

No, because we don't directly support cake php in this package.

josegonzalez commented 9 years ago

Just curious - and not trolling - would you merge in a pr that contains a single-class providing support to the CakePHP framework or other frameworks? With tests of course.

It would help people find this package when searching for cakephp packages.

GrahamCampbell commented 9 years ago

would you merge in a pr that contains a single-class providing support to the CakePHP framework or other frameworks? With tests of course.

Probably not. Laravel integration requires no extra classes, so I don't see why any other framework should either?

GrahamCampbell commented 9 years ago

I want to leave it up to users to write their own small integration to exactly suit their own test suites.

josegonzalez commented 9 years ago

Gotcha, makes sense to me :)

Along the same vein, would you be opposed to adding gourmet/muffin to the suggests option of factory-muffin?

philsturgeon commented 9 years ago

No, because we don't directly support cake php in this package.

You don't directly support Laravel either. If you do, you should stop that.

All packages in the PHP League are framework agnostic. Some of them historically have had a Laravel service provider, because a lot of us were using Laravel at the time and it was just one class, one service provider sat there enabling optional tighter Laravel integration.

They should be removed from any package that has them, for the same reason that you @GrahamCampbell just said you wouldn't want to support CakePHP directly.

Any framework - be it Laravel or CakePHP - that could benefit from tighter integration based on config files, service providers, whatever it is. should have a bridge package, and those packages should be linked to in the README.

Then you move on to saying:

Laravel integration requires no extra classes, so I don't see why any other framework should either?

If nothing in the code makes it work better with Laravel, then the package has nothing to do with Laravel. If "laravel" is a keyword to help Laravel users, then we should help Cake, Slim, Aura and Zend users too, right?

Seeing as adding all other popular frameworks is completely unnecessary, why not just remove the keyword and make sure the package is advertised and described more clearly for those who need to know what they're looking for.

Factory Girl has factory_girl_rails :)

frankdejonge commented 9 years ago

I'd be in favour of dropping the laravel keyword here too. Firstly because it's not really tied to the package. Secondly, because we, as an open group, should not favour one framework over another. This is, in my opinion, in contradiction with this group's values. If there's a package which provides a service provider of some sorts, then by all means. But that's not the case here. On the other hand... I'd like to see more framework binding being suggested either in the composer.json suggest section, but preferably in the documentation. This would be a nice display of how being "framework agnostic" doesn't mean it's not easy to use in within framework, but rather shows you that because of how we do thing, so much more people can benefit. Strengthening our values in the process.

Since this is probably going to be a recurring theme, as members of the group tend to run in different circles, it might be a good step to set some guidelines for this. I think it's great to see frameworks like CakePHP, who've been around for ages and are still very much relevant (checkout Cake 3, it's got some very nifty packages), to embrace our packages. I can also understand that if we favour or lean more towards a particular framework that can become a blocker in the adoption of a package.

Because of our non-framework nature, it's probably best to refrain from suggesting framework specifically UNLESS the package is a framework binding. Being framework agnostic is one of the foundations of The PHP League, let's figure out a way to be all-inclusive: Not a framework™.

tl;dr; I think the laravel keyword in this package doesn't belong here. It doesn't I think framework related keywords belong to bridging packages, which we can also provide as part of the league. I think we should be welcoming to all frameworks and stick with our goal to be framework agnostic.

jadb commented 9 years ago

@GrahamCampbell - should I re-open a new PR?

I believe everyone here has some valid points and if it's not dropping the Laravel keyword, then maybe a PR adding the CakePHP keyword (and probably other frameworks as well).

jadb commented 9 years ago

@GrahamCampbell - are you just ignoring because you don't see to agree? I was really hoping for a better outcome out of this discussion and definitely a little more inter-framework cooperation (just like I did on this PR).

/cc @philsturgeon @frankdejonge @josegonzalez

GrahamCampbell commented 9 years ago

If the feeling from the php league is that it should be changed, then they will change it whether I agree or not.

GrahamCampbell commented 9 years ago

I wasn't following this conversation because it's so trivial. Got more important things to be doing.

GrahamCampbell commented 9 years ago

You don't directly support Laravel either. If you do, you should stop that.

This package is designed only for laravel's database component Phil...

GrahamCampbell commented 9 years ago

The fact that it works with other libraries is just a happy coincidence that they've copied the way eloquent works, or vice versa.

philsturgeon commented 9 years ago

This package is designed only for laravel's database component Phil...

As I said in #392, if this has a hard dependency on Laravel then that dependency needs to be moved to a bridge package, just like factory_girl has factory_girl_rails. But, it seems like a soft dependency on eloquent (not Laravel), so once again, the Laravel keyword is completely irrelevant here.

I merged the other PR as the leadership group all agreed. Again, seems like a mountain out of a molehill, but it's a fundamental principle of the group to not favor any framework as that divides interest and support and causes untold bullshit to happen behind the scenes that I don't want to deal with.

GrahamCampbell commented 9 years ago

if this has a hard dependency on Laravel then that dependency needs to be moved to a bridge package

I don't think you're understand the entire concept of this package.

philsturgeon commented 9 years ago

I understand it perfectly, you're not quite getting my theoretical explanation that it's either Laravel specific and should be changed, or isn't Laravel specific and needs to lose the keyword.

GrahamCampbell commented 9 years ago

I think we both understand each other, we just don't agree with each other. I think we should keep the keyword so that people looking for laravel testing solutions can wind up here, but you think, why should we prioritise laravel over any other framework. :)

philsturgeon commented 9 years ago

Yeah that’s it. 

What about people who want CakePHP testing tools?

What about people who want SlimPHP testing tools?

What about people who want Aura testing tools?

What about people who want PHP testing tools?

It’s got nothing to do with the framework and we’re not going to list the lot, so advertise the tool itself as clearly as possible and write a blog post about it to catch the Laravel users. Done. :)

GrahamCampbell commented 9 years ago

Ok I guess.