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

Ensure compatibility between Recurly_Resource constructors #335

Merged
merged 1 commit into from
Oct 27, 2017
Merged

Ensure compatibility between Recurly_Resource constructors #335

merged 1 commit into from
Oct 27, 2017

Conversation

stevegrunwell
Copy link

Three sub-classes of Recurly_Resource (Recurly_Plan, Recurly_Addon, and Recurly_AccountBalance) use overload their parents' __construct() methods, but do not accept arguments. This makes it impossible to use mocked Recurly_Client objects during testing when working with these three resources, meaning tests can't run without actually calling the Recurly API.

This PR ensures that these resources can accept $href and $client arguments, passing them through to the parent constructor.

Three sub-classes of Recurly_Resource (Recurly_Plan, Recurly_Addon, and Recurly_AccountBalance) use overload their parents' __construct() methods, but do not accept arguments. This makes it impossible to use mocked Recurly_Client objects during testing when working with these three resources.

This commit ensures that these resources can accept $href and $client arguments, passing them through to the parent constructor.
@bhelx bhelx self-requested a review October 27, 2017 16:07
@bhelx
Copy link
Contributor

bhelx commented Oct 27, 2017

@stevegrunwell thanks for taking the time to fix this! I will take a look at it today.

@stevegrunwell
Copy link
Author

Fantastic, thank you!

Copy link
Contributor

@bhelx bhelx left a comment

Choose a reason for hiding this comment

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

Ended up being pretty straightforward. I double checked that the constructor params were correct and that there were no other resources needing this.

@bhelx bhelx merged commit 203918b into recurly:master Oct 27, 2017
@bhelx
Copy link
Contributor

bhelx commented Oct 27, 2017

@stevegrunwell thanks again!

@stevegrunwell stevegrunwell deleted the fix/recurly-resource-constructors branch October 27, 2017 16:56
@bhelx bhelx added the V2 V2 Client label Mar 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
V2 V2 Client
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants