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

[BC Break] Rework resource owners to use Symfony Http Client internally #1681

Merged
merged 1 commit into from Aug 17, 2021
Merged

[BC Break] Rework resource owners to use Symfony Http Client internally #1681

merged 1 commit into from Aug 17, 2021

Conversation

stloyd
Copy link
Collaborator

@stloyd stloyd commented Jan 1, 2021

No description provided.

@stloyd stloyd force-pushed the feature/sf-http-client branch 3 times, most recently from 0def73e to 17cf193 Compare January 2, 2021 15:05
@XWB
Copy link
Member

XWB commented Jan 2, 2021

Looks good to me.

@stloyd stloyd added the DX label Jan 2, 2021
@stloyd stloyd mentioned this pull request Jan 2, 2021
Closed
@stloyd stloyd force-pushed the feature/sf-http-client branch 2 times, most recently from 54d38dc to 601a256 Compare January 5, 2021 10:16
@sadikoff
Copy link

wow looks like a cool simplification =)

@stloyd stloyd added this to the 2.0 - Game changer milestone Jan 15, 2021
@ltlsquare
Copy link

When can we expect this change to be available in the master branche? this is a definite game changer.

@stloyd
Copy link
Collaborator Author

stloyd commented Jan 22, 2021

There are few things missing in this PR that need to be addressed, but I'm currently doing commercial work to pay taxes.

Feel free to share and/or sponsor me on GitHub so I will be able to spend more time on open-source work:
https://github.com/sponsors/stloyd

@stephanvierkant
Copy link
Collaborator

Thanks! This looks like a great improvement.

What else should be addressed in this PR? Do you need any help?

@stloyd
Copy link
Collaborator Author

stloyd commented Jul 1, 2021

Thanks! This looks like a great improvement.

What else should be addressed in this PR? Do you need any help?

This requires "jump-in" testing in real project, which currently I cannot provide, so if you're able to test it & give me a feedback this for sure would speedup the process 😄

@stephanvierkant
Copy link
Collaborator

stephanvierkant commented Jul 1, 2021 •
edited

I get this error:

HWI\Bundle\OAuthBundle\OAuth\ResourceOwner\AbstractResourceOwner::__construct(): Argument #3 ($name) must be of type string, array given

@stloyd stloyd force-pushed the feature/sf-http-client branch 2 times, most recently from a804c6e to 0c4b4c4 Compare July 1, 2021 08:46
@stloyd
Copy link
Collaborator Author

stloyd commented Jul 1, 2021

I get this error:

HWI\Bundle\OAuthBundle\OAuth\ResourceOwner\AbstractResourceOwner::__construct(): Argument #3 ($name) must be of type string, array given

Should be fixed, during weekend I will try to setup small project with at least Github & Twitter logins to validate the changes.

@XWB
Copy link
Member

XWB commented Jul 1, 2021

Once validated, we could create a 1.x branch and bump master to 2.x so we can merge this PR.

@stloyd stloyd force-pushed the feature/sf-http-client branch 5 times, most recently from 9d97858 to 820f9b9 Compare August 2, 2021 15:32
@stloyd stloyd requested a review from XWB August 2, 2021 15:32
@stloyd stloyd force-pushed the feature/sf-http-client branch 2 times, most recently from 93c722e to 390e765 Compare August 2, 2021 15:44
@stloyd stloyd force-pushed the feature/sf-http-client branch 5 times, most recently from f547e75 to eeea947 Compare August 4, 2021 16:27
@stloyd
Copy link
Collaborator Author

stloyd commented Aug 4, 2021

@XWB @stephanvierkant @sadikoff (and everyone else interested 😄) This is ready to be tested 🔥

sadikoff
sadikoff reviewed Aug 6, 2021
View reviewed changes
Copy link

@sadikoff sadikoff left a comment

Choose a reason for hiding this comment

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

Woo finaly! Nice work!

One minor comment from me =)

Resources/config/util.xml Outdated Show resolved Hide resolved
@sadikoff
Copy link

sadikoff commented Aug 6, 2021

@stloyd WDYT about more PHP 7.4 usage? As I see it's a minimal PHP version allowed, probably we may use more property types definitions.

@stloyd
Copy link
Collaborator Author

stloyd commented Aug 6, 2021

It's definitely not scope of this PR, but I have started some changes while making internal classes final officially, but it will be easier if we have this merged before it.

@sadikoff
Copy link

sadikoff commented Aug 6, 2021 •
edited

It's definitely not scope of this PR

Totally! It was a more theoretical question =)

@stloyd stloyd mentioned this pull request Aug 16, 2021
Closed
@stloyd stloyd merged commit 8b97e77 into hwi:master Aug 17, 2021
@stloyd stloyd deleted the feature/sf-http-client branch August 17, 2021 08:28
@Jean85 Jean85 mentioned this pull request Aug 25, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sadikoff sadikoff sadikoff left review comments

@XWB XWB XWB approved these changes

Assignees
No one assigned
Labels
DX Enhancement
Projects
None yet
Milestone
2.0 - Game changer
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@stloyd @XWB @sadikoff @ltlsquare @stephanvierkant