-
Notifications
You must be signed in to change notification settings - Fork 129
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
Move commands to bernard bundle #259
Comments
@sagikazarmark are you still ok with moving the commands to the bundle? I like the idea as symfony doesn't include commands in their separate components but includes them in bundles (frameworkBundle, webserverBundle, ...). But I don't know if anyone is using the commands as a standalone I can start pr's to move the commands around! |
Hm, dunno. One could use them in any application, not just symfony, but then it shouldn't take much to install the bundle as a regular package. Let's keep as much symfony related integration in one as possible, so let's move them, unless there are other arguments against it. |
I use the commands (w/symfony-console) in an Expressive app. Don't know if that counts as standalone. Moving them to a separate package would make them less discoverable. Fine for symfony, less so for a smaller library. But hey, just my 2c |
No you have a good point there! So maybe we shouldn't move the commands. @sagikazarmark maybe we should keep the consume and produce command in here and move the doctrine commands to the doctrine driver package when we split the repo drivers. what do you think? |
Leaving the commands here yields an optional dependency. It also means we have no control over the symfony version the user installs (since we don't enforce it via Composer). We could add packages to the conflict section, but that's just not the solution. But I realized that moving them to the bundle might not be a good idea for "standalone" usage, since it requires half of Symfony to be installed (not an option). I don't have a nice solution, but I would still like to see them gone. |
Are they really useful here? It's not that much code, so if absolutely necessary, could be duplicated.
The text was updated successfully, but these errors were encountered: