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

Add symfony/var-dumper as a requirement? #15

Closed
javiereguiluz opened this issue Mar 22, 2018 · 7 comments · Fixed by #16
Closed

Add symfony/var-dumper as a requirement? #15

javiereguiluz opened this issue Mar 22, 2018 · 7 comments · Fixed by #16
Assignees
Labels
status:ready for adoption Feel free to implement this issue. type:enhancement Enhancement
Milestone

Comments

@javiereguiluz
Copy link
Contributor

What steps will reproduce the problem?

In this class: https://github.com/yiisoft/yii2-shell/blob/master/YiiCaster.php you import a Symfony class: use Symfony\Component\VarDumper\Caster\Caster; but you don't require symfony/var-dumper in the composer.json of this project: https://github.com/yiisoft/yii2-shell/blob/master/composer.json

This works by chance and pure luck. Some package is installing the package you need (symfony/var-dumper) so this works without you actually requiring that package.

What's expected?

symfony/var-dumper should be added to the require section of the composer.json file.

What do you get instead?

Additional info

Q A
Yii vesion Any
PHP version Any
Operating system Any
@schmunk42
Copy link

Makes sense.

We also should create a new release, since there was another requirement update for psy/psysh.
The latest stable release requires symfony/console 2.x

@samdark samdark added status:ready for adoption Feel free to implement this issue. type:enhancement Enhancement labels Mar 22, 2018
@samdark
Copy link
Member

samdark commented Mar 22, 2018

@javiereguiluz totally makes sense. Would you like to handle it in a pull request?

@cebe cebe added this to the 2.0.1 milestone Mar 22, 2018
samdark pushed a commit that referenced this issue Mar 22, 2018
* Added symfony/var-dumper requirement
* Defined the allowed dependency versions
@pana1990
Copy link
Contributor

@javiereguiluz @samdark @schmunk42 psy/psysh has already "symfony/var-dumper" dependency in her composer.json : https://github.com/bobthecow/psysh/blob/master/composer.json#L18

is it necessary to include it in our composer?

@schmunk42
Copy link

Yes, since it's used here. If they would remove the dep i.e. our code would break.

@javiereguiluz
Copy link
Contributor Author

@pana1990 a general rule is that if your own code uses some third-party code directly, you should add it to require or require-dev. In this case, Yii2 is explicitly importing a Symfony class in this line: https://github.com/yiisoft/yii2-shell/blob/master/YiiCaster.php

@pana1990
Copy link
Contributor

Of course, thanks for your reply :)

@bertoost
Copy link

bertoost commented Feb 16, 2020

@schmunk42 Why is a production package using VarDumper? This is a dev package and now it becomes a production package which also brings some other disadvantages because of this. Since I now can not use latest version of it in dev environments.
Therefore I cannot find the real usage of this Symfony component. Is it really necessary?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:ready for adoption Feel free to implement this issue. type:enhancement Enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants