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

perf: Expand reflected calls into Client methods #510

Merged
merged 1 commit into from
Jan 8, 2019

Conversation

kattrali
Copy link
Contributor

Goal

Improve performance and shrink callstacks by removing the reflection calls from Client to Configuration. Also beef up test coverage for config options!

Related to #476

Changeset

Removed the __call method from Client, replacing it with the instance methods from Configuration.

Tests

Added new unit tests for the return values and behaviors of Client to ensure that:

  • The setters which return the instance still return the right thing
  • The getters return the same object as before

The tests pass before and after the change.

Copy link
Contributor

@Cawllec Cawllec left a comment

Choose a reason for hiding this comment

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

Looks good: Checked all the methods have been covered, tests cover everything, and it works in a Laravel test application.

@kattrali kattrali merged commit cbaf17a into master Jan 8, 2019
@kattrali kattrali deleted the kattrali/remove-reflection branch January 8, 2019 15:41
@kattrali kattrali mentioned this pull request May 24, 2019
15 tasks
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