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

Replace pimple with laravel's dependency injection container #9913

Closed
touhidurabir opened this issue Apr 25, 2024 · 6 comments
Closed

Replace pimple with laravel's dependency injection container #9913

touhidurabir opened this issue Apr 25, 2024 · 6 comments
Assignees
Labels
Housekeeping:1:Todo Any dependency management or refactor that would be nice to have some day.
Milestone

Comments

@touhidurabir
Copy link
Member

touhidurabir commented Apr 25, 2024

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

  1. PKP\core\PKPServices
  2. APP\core\Services
  3. APP\services\OJSServiceProvider
  4. APP\services\OMPServiceProvider
  5. 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 --> #10177
ojs --> pkp/ojs#4358
omp --> pkp/omp#1633
ops --> pkp/ops#728
crossref-ojs --> pkp/crossref-ojs#51
jatsTemplate --> pkp/jatsTemplate#45
oaiJats --> pkp/oaiJats#45
plagiarism --> pkp/plagiarism#61
crossref-ops --> pkp/crossref-ops#41

@touhidurabir touhidurabir added the Housekeeping:1:Todo Any dependency management or refactor that would be nice to have some day. label Apr 25, 2024
@jonasraoni
Copy link
Contributor

I think this can be closed as a duplicate of #7131

@touhidurabir touhidurabir added this to the 3.5 Internal milestone Jul 9, 2024
@touhidurabir touhidurabir self-assigned this Jul 9, 2024
touhidurabir added a commit to touhidurabir/pkp-lib that referenced this issue Jul 9, 2024
touhidurabir added a commit to touhidurabir/ojs that referenced this issue Jul 9, 2024
touhidurabir added a commit to touhidurabir/ojs that referenced this issue Jul 9, 2024
@touhidurabir
Copy link
Member Author

@asmecher can you check the PRs at #9913 (comment). 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 .

@asmecher
Copy link
Member

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 #9276.

touhidurabir added a commit to touhidurabir/pkp-lib that referenced this issue Jul 11, 2024
touhidurabir added a commit to touhidurabir/ojs that referenced this issue Jul 11, 2024
touhidurabir added a commit to touhidurabir/pkp-lib that referenced this issue Jul 11, 2024
touhidurabir added a commit to touhidurabir/pkp-lib that referenced this issue Jul 11, 2024
touhidurabir added a commit to touhidurabir/ojs that referenced this issue Jul 11, 2024
touhidurabir added a commit to touhidurabir/ojs that referenced this issue Jul 11, 2024
touhidurabir added a commit to touhidurabir/ojs that referenced this issue Jul 11, 2024
touhidurabir added a commit to touhidurabir/omp that referenced this issue Jul 11, 2024
touhidurabir added a commit to touhidurabir/omp that referenced this issue Jul 11, 2024
touhidurabir added a commit to touhidurabir/ops that referenced this issue Jul 11, 2024
touhidurabir added a commit to touhidurabir/ops that referenced this issue Jul 11, 2024
touhidurabir added a commit to touhidurabir/pkp-lib that referenced this issue Aug 1, 2024
touhidurabir added a commit to touhidurabir/pkp-lib that referenced this issue Aug 1, 2024
touhidurabir added a commit to touhidurabir/ojs that referenced this issue Aug 1, 2024
touhidurabir added a commit to touhidurabir/ojs that referenced this issue Aug 1, 2024
touhidurabir added a commit to touhidurabir/ojs that referenced this issue Aug 1, 2024
touhidurabir added a commit to touhidurabir/omp that referenced this issue Aug 1, 2024
touhidurabir added a commit to touhidurabir/omp that referenced this issue Aug 1, 2024
touhidurabir added a commit to touhidurabir/omp that referenced this issue Aug 1, 2024
touhidurabir added a commit to touhidurabir/ops that referenced this issue Aug 1, 2024
touhidurabir added a commit to touhidurabir/ops that referenced this issue Aug 1, 2024
touhidurabir added a commit to touhidurabir/ops that referenced this issue Aug 1, 2024
@touhidurabir
Copy link
Member Author

@asmecher

  1. rebased
  2. all tests are passing for OJS, OMP and OPS .
  3. Only plagiarism had missing stable-3_4_0 branch and I have created it from main .

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 .

asmecher pushed a commit that referenced this issue Aug 1, 2024
* #9913 Removed pimple container dependency

* #9913 removed usage of Services::get and related namespaces

* #9913 removed Services::get reference from older upgrade migrations
asmecher pushed a commit to pkp/ojs that referenced this issue Aug 1, 2024
asmecher pushed a commit to pkp/omp that referenced this issue Aug 1, 2024
asmecher pushed a commit to pkp/ops that referenced this issue Aug 1, 2024
asmecher added a commit to pkp/crossref-ojs that referenced this issue Aug 1, 2024
pkp/pkp-lib#9913 replaced depreciated Services::get with app()->get
asmecher pushed a commit to pkp/jatsTemplate that referenced this issue Aug 1, 2024
asmecher pushed a commit to pkp/oaiJats that referenced this issue Aug 1, 2024
asmecher added a commit to pkp/plagiarism that referenced this issue Aug 1, 2024
pkp/pkp-lib#9913 replaced depreciated Services::get with app()->get
asmecher added a commit to pkp/crossref-ops that referenced this issue Aug 1, 2024
pkp/pkp-lib#9913 replaced depreciated Services::get with app()->get
@asmecher
Copy link
Member

asmecher commented Aug 1, 2024

All merged, thanks! Closing.

@asmecher asmecher closed this as completed Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Housekeeping:1:Todo Any dependency management or refactor that would be nice to have some day.
Projects
None yet
Development

No branches or pull requests

4 participants