search

leomarquine / php-etl

Extract, Transform and Load data using PHP.
MIT License
178 stars 81 forks source link

#28 Removed service container injection #29

Closed ecourtial closed 4 years ago

ecourtial commented 4 years ago

Related to issue #28 So after an investigation, they are some major issues, related to DI, in the current version of the ETL.

  1. While the ETL is supposed to be usable in any PHP application, it embeds some own DI mechanism (Laravel extension) which may - in some case - trigger incompatibility issues when using custom Extractors/Transformers/Loaders

  2. DI injection is done in a dangerous way. We inject the service container (or worst, call it directly)

  3. DI is not used when getting the Extractors/Transformers/Loaders. Instead, the developer call it via an alias (as string), then the code uses some binding stuff to call the service container and get the good component.

The PR does the following:

Side notes:

ecourtial commented 4 years ago

@leomarquine Any feedback?

leomarquine commented 4 years ago

I was considering ditch the service container, but in the next major release. Now it is a breaking change.

ecourtial commented 4 years ago

Hi @leomarquine

I was considering ditch the service container, but in the next major release. Now it is a breaking change.

Then perfect :) . Yes as I said it is a BC. I think it should be nice to do it ASAP. The code is ready, if you "pre-aprove" this PR' I will update the documentation too in the same PR :)

ecourtial commented 4 years ago

Hi @leomarquine So what should we do? Is it ok for you so I update the documentation in the same PR?

ecourtial commented 4 years ago

Closing. We decided to fork to project and make it evolves in its own way.