elgentos / LargeConfigProducts

Large Configurable Products workaround for Magento 2
91 stars 28 forks source link

Package suggested used as dependency #16

Closed giuseppemorelli closed 5 years ago

giuseppemorelli commented 5 years ago

Hi @peterjaap , if you try to compile the code in production mode you'll have this error:

image

Magento CE 2.2.5 PHP 7.0

In your composer.json you suggest to install the package renatocason/magento2-module-mq but in fact you use that package in this file: https://github.com/elgentos/LargeConfigProducts/blob/master/Model/Consumer.php so we need to install, not only if we want.

What if we put renatocason/magento2-module-mq as dependency and not as suggestion?

Thank you very much

peterjaap commented 5 years ago

Ah sorry totally missed that!

Would you say that everybody wants to use the message queue? EE users might want to use Rabbitmq instead of this one?

giuseppemorelli commented 5 years ago

I mean that is not possibile with the CE to use your module without install renatocason/magento2-module-mq because I have this:


namespace Elgentos\LargeConfigProducts\Model;
use Elgentos\LargeConfigProducts\Model\Prewarmer;
use Rcason\Mq\Api\ConsumerInterface;  // ******* <- dependency
use Symfony\Component\Process\Process;
use Psr\Log\LoggerInterface;
class Consumer implements ConsumerInterface
{
    /**
     * @var LoggerInterface
     */
    protected $logger;
    /**
     * @var Prewarmer
     */
    private $prewarmer;

......

in this file: https://github.com/elgentos/LargeConfigProducts/blob/master/Model/Consumer.php

So it's a requirement that I cannot choose. I've just open a pull request about that.

Hope that I explained well :)

peterjaap commented 5 years ago

Yes I understood but I was trying to figure out a way to circumvent the dependency and still make it optional. In the meantime, I've merged your PR.