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

Update composer.json #4720

Closed
wants to merge 1 commit into from
Closed

Update composer.json #4720

wants to merge 1 commit into from

Conversation

joelharkes
Copy link

Tinker should be a dev dependency. since its only used by developers and not in production i would say?

Tinker should be a dev dependency. since its only used by developers and not in production i would say?
@GrahamCampbell
Copy link
Member

Thanks for the PR, but this has been proposed before, and the feeling was that tinker still has a use case in production environments, for probing the state of the production server. You are of course free to move this into dev in your applications. :)

@joelharkes
Copy link
Author

So is it than used in production out of the box or not? (why not let users move it to require-dev when they want to start using it in production?)

@joelharkes joelharkes deleted the patch-1 branch August 1, 2018 13:39
@robclancy
Copy link

What? Laravel is a framework. It doesn't tell you how to use things. Tinker is useful on production as well as development and that's the default state. But like everything in the framework you can change it.

@SjorsO
Copy link
Contributor

SjorsO commented Aug 8, 2018

Although tinkering in production can be useful, I feel like tinkering in any serious production application is not a good practice. Moving Tinker to the dev-dependencies could help show users that tinkering in production is something you should try to avoid.

@joelharkes
Copy link
Author

@SjorsO is what I was thinking.

Also faker, mockery and whoops library can have production purposes too, but they are a dev dependency.

@fireynis
Copy link

fireynis commented Aug 8, 2018

This is one of those things that you like, and I'm sure other people don't like. So in this scenario there is no reason to make the change, especially since it can be done in your own projects.

So the thing to do here is leave it as is.

@joelharkes
Copy link
Author

I'm just saying the given reason is flawed.

If you say we include it for a specific feature or because we see it being used in production environments or because it's a tiny package with almost no overhead. Those are more logical reasons.

@fireynis
Copy link

fireynis commented Aug 8, 2018

My point is that currently the setting in the library works for some people and not others. If we change it then someone will open a PR saying, I wish tinker was in required and we will have the reverse conversation. This is the way it is, it is not core code so you can change it and make it how you want in your own project. As a setting though changing it now will just cause the inverse conversation to happen. No gain.

@robclancy
Copy link

If you can't workout what is good in production from common sense and not from a field in a json file then you shouldn't have access to production.

And why would you use faker or mockery in production... lmao. Then there is whoops, a security issue if it was running. So your examples are just bad.

Modify your composer.json like you are meant to and stop asking to hold yours and everyone elses hands. Do your jobs.

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

Successfully merging this pull request may close these issues.

5 participants