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

Increase test coverage + a few improvements #14

Merged
merged 6 commits into from
Feb 10, 2020
Merged

Increase test coverage + a few improvements #14

merged 6 commits into from
Feb 10, 2020

Conversation

edigu
Copy link
Contributor

@edigu edigu commented Feb 9, 2020 •
edited
Loading

This PR increase coverage of CacheFactoryTest with a few more additional improvements.

Notable changes are:

  • Test behavior with wrapped instances (Memcached only, Predis could be also introduced in same way)
  • Marked all exceptions from library namepace as final, introduced named constructors whenever possible.
  • Reduced array_key_exists calls and variable assignments in a few places. (Performance++)
  • Some of the tests were marked as final previously, remaining ones marked as well for consistency.

Ocramius
Ocramius approved these changes Feb 10, 2020
View reviewed changes
Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @edigu!

@Ocramius Ocramius added this to the 2.0.0 milestone Feb 10, 2020
@Ocramius Ocramius added the enhancement New feature or request label Feb 10, 2020
@Ocramius Ocramius self-assigned this Feb 10, 2020
@Ocramius Ocramius added the BC BREAK This PR contains a BC break; major version should be incremented label Feb 10, 2020
@Ocramius Ocramius merged commit 554520a into Roave:master Feb 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ocramius Ocramius Ocramius approved these changes

Assignees

@Ocramius Ocramius

Labels
BC BREAK This PR contains a BC break; major version should be incremented enhancement New feature or request
Projects
None yet
Milestone
2.0.0
Development

Successfully merging this pull request may close these issues.
2 participants
@edigu @Ocramius