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

Now that Guzzle7 is out use the guzzle7 adapter #5

Merged
merged 4 commits into from
Sep 29, 2020

Conversation

swilla
Copy link
Contributor

@swilla swilla commented Sep 29, 2020

I was getting errors trying to install this package with the Guzzle6 adapter, I was able to get everything working bu upgrading to Laravel 8 (and Guzzle 7).

This PR just bumps the guzzle adapter to 7.

I was able to successfully get this working on a Laravel 8 package using the following command:

composer require jdecool/clockify-api php-http/guzzle7-adapter

@jdecool jdecool self-assigned this Sep 29, 2020
@jdecool jdecool self-requested a review September 29, 2020 04:46
@jdecool jdecool removed their assignment Sep 29, 2020
Copy link
Owner

@jdecool jdecool left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @swilla

This PR make me realize that we should not constraint any guzzle version in the Composer.
I use php-http/httplug to be independant of any http client library. If we need to bump Guzzle because your application use a specific version, it's a mistake.

So I suggest to simply remove the guzzle dependency in the composer.json and let the user and the application tell which client to use.

@swilla
Copy link
Contributor Author

swilla commented Sep 29, 2020

@jdecool I removed the guzzle constraint. For the record, it is still working in my application. I have also updated the readme.

Copy link
Owner

@jdecool jdecool left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now I remember why I've had guzzle, it's because a client is required for testing and install the library on your development machine.

I think we could php-http/mock-client which is designed for testing and tests will be green.

Copy link
Owner

@jdecool jdecool left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice.

Thanks @swilla

@jdecool jdecool merged commit 9a342ee into jdecool:master Sep 29, 2020
@jdecool
Copy link
Owner

jdecool commented Sep 29, 2020

I just tag a new release with this PR.

See https://github.com/jdecool/clockify-api/releases/tag/0.2.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants