-
Notifications
You must be signed in to change notification settings - Fork 51
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
Reduction of external dependencies #78
Comments
IMO keep monolog and console. Yaml isn't necessary and we can change the config to be a straight PHP array instead quite easily. I agree with your assessment. Monolog is quite ubiquitous and overall pretty light so it doesn't really hurt to keep (unless other projects require a different version but we should be flexible here) |
Since this suggestion is my work:
My use case for this library is a docker container with a customized boot-script tho. The symfony/console is useless for me, as is the forking. Monolog just passes through to stdout, i never will use symfony/process, yaml is nice, but not necessary. update: |
+1 for :
I suggest, following @scones suggestions, to move all commands into another repository. For examle, the start command implementation should be free... it can be reduced to a
A worker is nothing more. On the other hand, more repositories, more complexity... |
@mjphaynes Should we create a Users who are currently using "require": {
"mjphaynes/php-resque": "~2.1"
} would need to change this to "require": {
"mjphaynes/php-resque": "~3.0",
"mjphaynes/php-resque-cli": "~1.0"
} What do you think? Thank you, best regards |
you can remove |
Ah, yes of course 😀 |
Move symfony/process to an optional dependency (required for dev, suggested otherwise). This is a preparation for #78.
Move symfony/process to an optional dependency (required for dev, suggested otherwise). This is a preparation for #78.
As I cannot create a new project here, the PHP and Symfony version dump is done with php-resque v3 🥳 |
As part of the discussion in #76, @scones suggested to reconsider and possibly slim down external dependencies. The PR #71 deals already with the Symfony component versions, however the scope of this issue goes beyond that.
In general, I like the idea of reducing unnecessary dependencies so I did some research on the current state.
External dependencies besides predis are at the moment (current master):
monolog/monolog ~1.7
symfony/console ~2.7|~3.0
symfony/yaml ~2.7|~3.0
symfony/process ~2.7|~3.0
As far as I can see, the speed test is the only place where
symfony/process
is currently used. So this dep could be moved to suggest and the speed test could check if it is available.The core functionality for enqueuing jobs would only require
symfony/yaml
(for parsing the resque configuration) andmonolog/monolog
(for logging). Both could be refactored to avoid those external dependencies as well. In the case of the logger this would probably be quite a bit of work. As monolog itself has no dependencies beside psr/log, the question is if that makes sense. For the YAML parsing, it might be beneficial to extract the configuration loading to allow e.g. loading from an array or other sources as well. However, installing the symfony/yaml component by default would probably still be a good idea.symfony/console
would remain as it is required by the commands. The question is whether extracting all of the commands to a separate project is worth the time. According to the Symfony 4.0 and the preliminary master/5.0 upgrade guides, the effort to work with all supported versions of symfony/console is small.To examine the impacts, benefits and drawbacks of such changes, I'd like to discuss the issue with the most active contributors of the past months, and of course with the project owner.
@mjphaynes @francislavoie @iam-merlin @MaximeMaillet @scones What do you think? Should we consider such a refactoring for the next major release?
The text was updated successfully, but these errors were encountered: