Skip to content

Decouple from Pimple #50

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.

Sign up for GitHub

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

Jump to bottom
Open
bradjones1 opened this issue Jul 12, 2022 · 5 comments · May be fixed by #52 or #54
Open

Decouple from Pimple #50

bradjones1 opened this issue Jul 12, 2022 · 5 comments · May be fixed by #52 or #54

Comments

@bradjones1
Copy link

Thanks for this handy package. Pimple development has ended/is in maintenance mode, and I don't necessarily think this package need be aware of the dependency injection container per se. Open to decoupling from pimple/pimple and refactoring to allow dependencies like the HTTP client be pulled in as a dependency from your DIC of choice?
@gnello
Copy link
Owner

gnello commented Jul 14, 2022

Hi @bradjones1, thanks for forcing me to think about this.
You're totally right, this package should follow a more SOLID approach.

I'm thinking of changing the constructor as follows

public function __construct(array $options[])

rather than

public function __construct(Psr\Container\ContainerInterface $container)

What do you think? Do you have any suggestions?

akuzia added a commit to akuzia/php-mattermost-driver that referenced this issue Mar 6, 2023
@akuzia
ref gnello#50 decouple from pimple
6d94e7c
@akuzia
Copy link

akuzia commented Mar 6, 2023 •
edited
Loading

@gnello I've made a fork removing pimple and switching from Guzzle to any PSR7/17/18 compatible http client implementation. If you are ok with this changes I'm willing to continue working on that in next several weeks until changes are merged to the upstream.

akuzia added a commit to akuzia/php-mattermost-driver that referenced this issue Mar 10, 2023
@akuzia
ref gnello#50 decouple from pimple
cf5cadc
@tacman
Copy link

tacman commented May 2, 2023

both those changes sound great -- can you also update the documentation to use the Symfony HttpClient instead of guzzle? And if it's ready, submit a PR, since having guzzle and pimple just for this library seems a bit heavy. Thanks.

@akuzia
Copy link

akuzia commented May 3, 2023

@tacman I've updated README & added #54

This was linked to pull requests May 9, 2023
#50: remove pimple/pimple container #52
Open
switch from GuzzleHttp to psr 7/17/18 #54
Open
@gnello
Copy link
Owner

gnello commented May 9, 2023

Hi @akuzia, @tacman,
these are very good changes.

I think we should bump the version and make all possible improvements before tagging the 3.0.0, e.g. require php8.
I will check the pull requests and look for other possible breaking changes to bring in.

@gnello gnello linked a pull request May 9, 2023 that will close this issue
Open
akuzia added a commit to akuzia/php-mattermost-driver that referenced this issue May 10, 2023
@akuzia
ref gnello#50 decouple from pimple
03d7fc0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@tacman @bradjones1 @akuzia @gnello