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.

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

Deprecate Guzzle with psr http-client #1664

Merged
merged 1 commit into from
Oct 9, 2020
Merged

Deprecate Guzzle with psr http-client #1664

merged 1 commit into from
Oct 9, 2020

Conversation

core23
Copy link
Member

@core23 core23 commented Jan 3, 2020 •
edited
Loading

I am targetting this branch, because this adds a new implementation without removing the existing one. This is a rework of #1181, but targets the psr http-client.

Changelog

### Added
- Added support for `psr/http-client` in `BaseVideoProvider`

### Deprecated
 - Deprecate `Guzzle` and `Buzz` usage in `BaseVideoProvider`

Subject

Added support for an abstract http client without removing the existing old guzzle implementation.

@core23 core23 force-pushed the http-client branch 2 times, most recently from dd75668 to 002cb4b Compare January 3, 2020 22:04
phansys
phansys requested changes Jan 4, 2020
View reviewed changes
UPGRADE-3.x.md Outdated Show resolved Hide resolved
src/Client/GuzzleClient.php Outdated Show resolved Hide resolved
src/Provider/BaseVideoProvider.php Outdated Show resolved Hide resolved
tests/Provider/VimeoProviderTest.php Outdated Show resolved Hide resolved
tests/Provider/VimeoProviderTest.php Outdated Show resolved Hide resolved
tests/Provider/VimeoProviderTest.php Outdated Show resolved Hide resolved
tests/Provider/VimeoProviderTest.php Outdated Show resolved Hide resolved
tests/Provider/VimeoProviderTest.php Outdated Show resolved Hide resolved
tests/Provider/VimeoProviderTest.php Outdated Show resolved Hide resolved
@phansys phansys added the minor label Jan 4, 2020
@SonataCI
Copy link
Collaborator

SonataCI commented Jan 5, 2020

Could you please rebase your PR and fix merge conflicts?

@github-actions github-actions bot closed this Jan 31, 2020
@core23 core23 reopened this Jan 31, 2020
@SonataCI
Copy link
Collaborator

Could you please rebase your PR and fix merge conflicts?

@SonataCI
Copy link
Collaborator

Could you please rebase your PR and fix merge conflicts?

@core23 core23 force-pushed the http-client branch 6 times, most recently from 9059657 to 4e149ec Compare July 2, 2020 17:02
@SonataCI
Copy link
Collaborator

Could you please rebase your PR and fix merge conflicts?

@core23 core23 force-pushed the http-client branch 3 times, most recently from ea1b0b4 to df43d23 Compare September 11, 2020 18:00
phansys
phansys requested changes Sep 11, 2020
View reviewed changes
docs/reference/advanced_configuration.rst Outdated Show resolved Hide resolved
docs/reference/installation.rst Outdated Show resolved Hide resolved
src/Client/BuzzClient.php Show resolved Hide resolved
src/Provider/BaseVideoProvider.php Outdated Show resolved Hide resolved
src/Provider/BaseVideoProvider.php Outdated Show resolved Hide resolved
@core23 core23 force-pushed the http-client branch 4 times, most recently from a5a4008 to 21b066b Compare September 11, 2020 18:54
@core23 core23 marked this pull request as ready for review September 11, 2020 18:54
phansys
phansys requested changes Sep 11, 2020
View reviewed changes
docs/reference/installation.rst Outdated Show resolved Hide resolved
src/Provider/BaseVideoProvider.php Outdated Show resolved Hide resolved
src/Provider/BaseVideoProvider.php Outdated Show resolved Hide resolved
phansys
phansys requested changes Sep 14, 2020
View reviewed changes
tests/Provider/VimeoProviderTest.php Outdated Show resolved Hide resolved
tests/Provider/VimeoProviderTest.php Outdated Show resolved Hide resolved
tests/Provider/YouTubeProviderTest.php Outdated Show resolved Hide resolved
tests/Provider/YouTubeProviderTest.php Outdated Show resolved Hide resolved
phansys
phansys requested changes Sep 14, 2020
View reviewed changes
tests/Provider/YouTubeProviderTest.php Outdated Show resolved Hide resolved
tests/Provider/YouTubeProviderTest.php Outdated Show resolved Hide resolved
tests/Provider/YouTubeProviderTest.php Outdated Show resolved Hide resolved
tests/Provider/YouTubeProviderTest.php Outdated Show resolved Hide resolved
tests/Provider/YouTubeProviderTest.php Outdated Show resolved Hide resolved
tests/Provider/YouTubeProviderTest.php Outdated Show resolved Hide resolved
tests/Provider/VimeoProviderTest.php Outdated Show resolved Hide resolved
tests/Provider/VimeoProviderTest.php Outdated Show resolved Hide resolved
@core23 core23 force-pushed the http-client branch 2 times, most recently from 7466e7f to beb38d1 Compare September 17, 2020 06:26
phansys
phansys previously approved these changes Sep 17, 2020
View reviewed changes
@SonataCI
Copy link
Collaborator

Could you please rebase your PR and fix merge conflicts?

@SonataCI
Copy link
Collaborator

SonataCI commented Oct 3, 2020

Could you please rebase your PR and fix merge conflicts?

@core23 core23 requested a review from a team October 3, 2020 18:10
phansys
phansys previously approved these changes Oct 3, 2020
View reviewed changes
docs/reference/advanced_configuration.rst
buzz:
connector: sonata.media.buzz.connector.file_get_contents # sonata.media.buzz.connector.curl

http:
client: 'symfony_http_client' # You need symfony/http-client for this
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
client: 'symfony_http_client' # You need symfony/http-client for this
client: 'symfony_http_client' # You need symfony/http-client for this

Copy link
Member Author

Choose a reason for hiding this comment

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

Looking at the other configs, we use indention in most cases

Copy link
Member

Choose a reason for hiding this comment

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

I saw in most of the recent PR that contributors are removing indentations.

Maybe the best things to do would be to decide about a standard.

Copy link
Member

Choose a reason for hiding this comment

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

I vote for the unaligned version, since it reduces unrelated merge conflicts.

@core23 core23 requested a review from a team October 6, 2020 06:31
jordisala1991
jordisala1991 reviewed Oct 6, 2020
View reviewed changes
composer.json Outdated Show resolved Hide resolved
jordisala1991
jordisala1991 reviewed Oct 6, 2020
View reviewed changes
composer.json Outdated Show resolved Hide resolved
@core23 core23 merged commit d2e1342 into sonata-project:3.x Oct 9, 2020
@core23 core23 deleted the http-client branch October 9, 2020 11:06
@michealmouner michealmouner mentioned this pull request Nov 4, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@greg0ire greg0ire greg0ire left review comments

@phansys phansys phansys left review comments

@jordisala1991 jordisala1991 jordisala1991 approved these changes

@VincentLanglet VincentLanglet VincentLanglet approved these changes

@OskarStark OskarStark Awaiting requested review from OskarStark

Assignees
No one assigned
Labels
minor
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@core23 @SonataCI @greg0ire @OskarStark @jordisala1991 @phansys @VincentLanglet