[go: up one dir, main page]

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

ControllerCommand uses Nette\PhpGenerator but that dependency is not declared in composer.json #604

Closed
lubiana opened this issue Aug 27, 2024 · 3 comments
Labels

Comments

@lubiana
Copy link
lubiana commented Aug 27, 2024

Describe the bug
A clear and concise description of what the bug is.

To Reproduce
Steps to reproduce the behavior:

  • clone current git master
  • composer install
  • composer test

Expected behavior
The tests run successfully

Actual behavior
ControllerCommandTest fails, and notes that the files does not exist.
When stepdebugging we can find out, that it boils down to a class not found error for Nette\PhpGenerator

Environment (please complete the following information):

  • OS: Linux
  • PHP Version: 8.3
  • Flight Version: current master
@lubiana lubiana added the Bug label Aug 27, 2024
@n0nag0n
Copy link
Collaborator
n0nag0n commented Aug 28, 2024

So I see your point, but here me out on this one.

It is required through require-dev in composer through flightphp/runway. Why I thought that was ok was typically you aren't going to be running runway commands in production, but instead on your development environments. If the argument is that runway should also work in production then we probably should move that flightphp/runway to production, but that would add a dependency to the core. One other option is that we could put the ControllerCommand and the other commands in runway itself, or in another package altogether to keep core dependency free.

Thoughts?

@lubiana
Copy link
Author
lubiana commented Aug 28, 2024

the current composer.json in the master branch requires runway in version 0.2.0, the php-generator dependency was only added in runway 0.2.2, so maybe we should bump the dependency requirement for runway to a newer version?

edit:
and yes, i am absolutety fine with requiring runway (and therefore php-generator), but we need to make shure, that the tests do not fail when installing the dependencies as declared in composer.json

@n0nag0n
Copy link
Collaborator
n0nag0n commented Aug 29, 2024

This is now updated and pushed to master. Thanks so much!

@n0nag0n n0nag0n closed this as completed Aug 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants