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

Reduction of external dependencies #78

Closed
xelan opened this issue Dec 28, 2017 · 7 comments
Closed

Reduction of external dependencies #78

xelan opened this issue Dec 28, 2017 · 7 comments

Comments

@xelan
Copy link
Collaborator

xelan commented Dec 28, 2017

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) and monolog/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?

@francislavoie
Copy link
Contributor

francislavoie commented Dec 28, 2017

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)

@scones
Copy link
Contributor

scones commented Dec 29, 2017

Since this suggestion is my work:

  • monolog could be condensed to psr/log, so it can be chosen by the library user.
  • symfony/console has nothing to do with queueing jobs, besides the act of queueing (and other helper tasks). Any funtionality (read: logic) should be moved to this library. The rest can be an own repository for bootstrapping with symfony/console.
  • symfony/yaml is not really necessary for configurstion
  • symfony/process - if its only for speed testing, it surely could be removed

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.
So my point of view is quite narrow.

update:
addendum.
When i want to include this project as dependency in other projects in order to spawn jobs, i might have ( and probably will) have dependency conflicts with the versions of symfony & monolog.
Since the spawning projects might just be able to spawn Jobs, but not to handle the jobs, those dependencies are quite a show stopper. To make this work, a smaller approach would be needed, that can just create Jobs, but not handle them.
Getting rid of the mentioned dependencies would be a huge step in that direction.

@merlindorin
Copy link
Contributor

+1 for :

  • psr/log
  • symfony/yaml (I would rather prefer env vars... maybe my nodejs background is talking)
  • symfony/process
  • symfony/console

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 sample.php file like:

require autoload.php;

// Create worker instance
$worker = new Resque\Worker();
$worker->work();

A worker is nothing more.

On the other hand, more repositories, more complexity...

@xelan
Copy link
Collaborator Author

xelan commented Jan 15, 2018

@mjphaynes Should we create a mjphaynes/php-resque-cli repository to reduce the dependencies and move the command stuff there? Of course this would be a significant BC break and require a major version bump.

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
Andreas

@merlindorin
Copy link
Contributor

you can remove mjphaynes/php-resque as dep... mjphaynes/php-resque-cli already have it ^^

@xelan
Copy link
Collaborator Author

xelan commented Jan 16, 2018

Ah, yes of course 😀

xelan added a commit that referenced this issue Oct 7, 2020
Move symfony/process to an optional dependency (required for dev, suggested otherwise).
This is a preparation for #78.
xelan added a commit that referenced this issue Oct 7, 2020
Move symfony/process to an optional dependency (required for dev, suggested otherwise).
This is a preparation for #78.
@xelan
Copy link
Collaborator Author

xelan commented Jan 3, 2023

As I cannot create a new project here, the PHP and Symfony version dump is done with php-resque v3 🥳

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

No branches or pull requests

4 participants