pkp-lib
pkp-lib copied to clipboard
Replace pimple with laravel's dependency injection container
Describe the proposal There are some implementations in the codebase that use the Pimple for the purpose of dependency injection. This package is a part of micro framework Silex which as been deprecated quite some times ago. Also as we have laravel's own dependency injection container, it's better to use that as we are adopting more from laravel's toolset .
What application are you using? OJS, OMP or OPS version main (3.5.0 pre release)
Additional information Only few classes are using it such as
- PKP\core\PKPServices
- APP\core\Services
- APP\services\OJSServiceProvider
- APP\services\OMPServiceProvider
- APP\services\OPSServiceProvider
Probably a good approach is to utilize the app or shared lib(pkp-lib) specific AppServiceProvider to put the functionalities there .
PRs pkp-lib --> https://github.com/pkp/pkp-lib/pull/10177 ojs --> https://github.com/pkp/ojs/pull/4358 omp --> https://github.com/pkp/omp/pull/1633 ops --> https://github.com/pkp/ops/pull/728 crossref-ojs --> https://github.com/pkp/crossref-ojs/pull/51 jatsTemplate --> https://github.com/pkp/jatsTemplate/pull/45 oaiJats --> https://github.com/pkp/oaiJats/pull/45 plagiarism --> https://github.com/pkp/plagiarism/pull/61 crossref-ops --> https://github.com/pkp/crossref-ops/pull/41
I think this can be closed as a duplicate of https://github.com/pkp/pkp-lib/issues/7131
@asmecher can you check the PRs at https://github.com/pkp/pkp-lib/issues/9913#issue-2263387287. This simple PRs centralize all services to use the laravel container and remove pimple container . If all ok, I plan to remove all reference of Services::get with app()->get() and port it to OMP and OPS .
Looks good, @touhidurabir, please go ahead! Also document any code changes that might be necessary for 3rd parties in the top comment, and add a link to it in https://github.com/pkp/pkp-lib/issues/9276.
@touhidurabir, thanks, I've reviewed the PRs here and they look good. Could you...
- Rebase the PRs as necessary to resolve conflicts,
- Ensure that tests are passing,
- Ensure that all plugin repos have distinct
stable-3_4_0branches, and finally - Merge the PRs into
main?
@asmecher
- rebased
- all tests are passing for
OJS,OMPandOPS. - Only plagiarism had missing
stable-3_4_0branch and I have created it frommain.
it's ready for merge but I do not have write access to few plugins repo . Can you please merge it as it's better to have all merged at same time to avoid conflict .
All merged, thanks! Closing.