Skip to content

Replace HTTPlug factories by PSR-17 - #1184

Merged
Nyholm merged 2 commits into
geocoder-php:masterfrom
nicolas-grekas:psr17
Dec 15, 2023
Merged

Replace HTTPlug factories by PSR-17#1184
Nyholm merged 2 commits into
geocoder-php:masterfrom
nicolas-grekas:psr17

Conversation

@nicolas-grekas

@nicolas-grekas nicolas-grekas commented May 11, 2023

Copy link
Copy Markdown
Contributor

All interfaces of php-http/message-factory are now deprecated.
This PR provides a way to move to PSR-17 instead.

@nicolas-grekas
nicolas-grekas force-pushed the psr17 branch 4 times, most recently from 7c339e5 to 411e864 Compare May 11, 2023 12:33
nicolas-grekas
nicolas-grekas commented May 11, 2023
View reviewed changes
Comment thread src/Http/Provider/AbstractHttpProvider.php Outdated
@nicolas-grekas

nicolas-grekas commented May 11, 2023

Copy link
Copy Markdown
Contributor Author

Failures will be fixed once 4.4 will be tagged.

@norkunas

norkunas commented Jun 22, 2023

Copy link
Copy Markdown
Member

@jbelien could you review/merge this? :)

Nyholm
Nyholm approved these changes Jun 30, 2023
View reviewed changes

@Nyholm Nyholm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you.

If we do a new Major after this PR, then we can also remove the getMessageFactory() method instead of deprecate it.

Comment thread src/Http/Provider/AbstractHttpProvider.php Outdated
@jbelien

jbelien commented Jul 8, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

Thanks for the review @Nyholm
Good idea indeed to create a new major release so we can also remove getMessageFactory(). 👍

@jbelien jbelien linked an issue Jul 9, 2023 that may be closed by this pull request
Abandoned dependency php-http/message-factory #1187
Closed
jbelien
jbelien previously requested changes Jul 9, 2023
View reviewed changes

@jbelien jbelien left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you fix the 8 failing tests ? I had a quick look and it seems related to createRequest() function.

@nicolas-grekas

nicolas-grekas commented Aug 5, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Needs geocoder-php/provider-integration-tests#20 before I can move forward.

@jbelien

jbelien commented Aug 6, 2023

Copy link
Copy Markdown
Member

Needs geocoder-php/provider-integration-tests#20 before I can move forward.

Woops! Sorry, I've missed that one. Checking it now 👍

@ker0x

ker0x commented Sep 25, 2023

Copy link
Copy Markdown

Hello @nicolas-grekas @jbelien ! Is there any progress on this PR?

@jbelien

jbelien commented Sep 26, 2023

Copy link
Copy Markdown
Member

Hello @nicolas-grekas @jbelien ! Is there any progress on this PR?

I've just merged geocoder-php/provider-integration-tests#20 and created a new release ; @nicolas-grekas can now update this PR. 👍

@nicolas-grekas
nicolas-grekas force-pushed the psr17 branch 10 times, most recently from 68d716c to 162224a Compare September 27, 2023 10:58
@nicolas-grekas

nicolas-grekas commented Sep 27, 2023

Copy link
Copy Markdown
Contributor Author

PR rebased and ready. The two failures are related to the subtree splitting and need v4.6.0 of the common-http package to be released.

This was referenced Oct 2, 2023
Merged
Closed
@Chris53897

Chris53897 commented Nov 1, 2023

Copy link
Copy Markdown
Contributor

@jbelien Is there a planned release date for version 4.6 for https://github.com/geocoder-php/php-common-http

@Stadly

Stadly commented Nov 3, 2023

Copy link
Copy Markdown
Contributor

Looking forward to this being released :)

@ruudk

ruudk commented Dec 14, 2023

Copy link
Copy Markdown
Contributor

@Nyholm You are probably very busy, but do you know what is needed to move this PR forward? Thanks 💙

Nyholm
Nyholm approved these changes Dec 14, 2023
View reviewed changes

@Nyholm Nyholm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you.

I think it all looks good. Just one minor

Comment thread src/Http/composer.json Outdated
"php-http/message-factory": "^1.0.2",
"psr/http-message": "^1.0 || ^2.0",
"psr/http-message-implementation": "^1.0",
"php": "^7.4 || ^8.0",

@Nyholm Nyholm Dec 14, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks like an issue while rebasing. Please remove this line and support php8 only.

Nyholm
Nyholm approved these changes Dec 14, 2023
View reviewed changes

@Nyholm Nyholm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@jbelien I am happy to merge.

@jbelien
jbelien dismissed their stale review December 15, 2023 15:59

Unblock reviews!

@jbelien

jbelien commented Dec 15, 2023

Copy link
Copy Markdown
Member

@jbelien I am happy to merge.

Thanks a lot @Nyholm for taking care of it! 😄
Sorry last few months have been very busy.
I've dismissed my review so it's not blocking.

@Nyholm

Nyholm commented Dec 15, 2023

Copy link
Copy Markdown
Member

No worries. That is how it is sometimes.

Let's move forward with this =)

@Nyholm
Nyholm merged commit b89f986 into geocoder-php:master Dec 15, 2023
@Nyholm

Nyholm commented Dec 15, 2023

Copy link
Copy Markdown
Member

Thank you for the PR and the reviews

@nicolas-grekas
nicolas-grekas deleted the psr17 branch December 18, 2023 08:51
@ruudk ruudk mentioned this pull request Dec 18, 2023
Closed
@oscarotero oscarotero mentioned this pull request Aug 4, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Nyholm Nyholm Nyholm approved these changes
@jbelien jbelien jbelien left review comments

Assignees

No one assigned

Labels

improvement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Abandoned dependency php-http/message-factory

8 participants

@nicolas-grekas @norkunas @jbelien @ker0x @Chris53897 @Stadly @ruudk @Nyholm