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

Add variant and mocking support. Refactor to use value objects. #39

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

dshafik
Copy link
Contributor

@dshafik dshafik commented Feb 9, 2022

First, let me apologize for the size of this PR… it started as a need to remove the static methods in the Feature facade to enable mocking and then it yak shaved away from me. I was unable to untangle it into a sane commit history. I highly recommend reading the updated README, in particular the sections on Variants and Mocking. There is also copious amounts of new tests.

I recognize that the refactor around value object is a fundamental change to the package, so If this is not desired I understand, but I consider it required for mocking, and it was helpful for implementing variant support too.

[NEW] Mocking can now be achieved easily using Unleash::fake() similar to Laravel's Event::fake() or Bus::fake(). This is achieved using UnleashFake.
[NEW] Variant support. Get the correct variant based on the flag configuration using Unleash::variant('flag-name', 'default-value');
[CHANGE] To better support mocking and variants, the package has been updated to use value objects. This should be backwards compatible for the most part, with the implementation fo ArrayAccess, except for checks using is_array().
[CHANGE] The Laravel-like enabled(), disabled(), get() and all() methods are now the primary, with the standard Unleash methods as aliases
[CHANGE] Tests have been updated to better handle Config mocking, simplifying some of them dramatically.

@mikefrancis
Copy link
Collaborator

Hey Davey, thanks for this.

I'll be honest, it might take me some time to get around to this so I'm thinking about (not related to the PR) what steps I might be able to take to decentralise ownership of this package.

In the meantime, are you using the fork of this? If so I imagine you'll push bug fixes as and when?

@dshafik
Copy link
Contributor Author

dshafik commented Feb 10, 2022

@mikefrancis OSS maintenance is a thankless task; perhaps creating a laravel-unleash org and move this repo (keeping the package name the same), then you can add a few contributors as folks who can review and merge PRs as well as help with bug fixes — I'm happy to try and contribute in this way :)

As for the fork, yes, we'll be deploying the fork to production tomorrow so bug fixes will be a priority for us!

@mikefrancis
Copy link
Collaborator

@dshafik That's what I'm thinking - it's great that something that started as a little hack is something which is getting used commercially but feeling the strain!

I'll set something up soon 👍 Thanks for your help.

[NEW] Mocking can now be achieved easily using `Unleash::fake()` similar to Laravel's `Event::fake()` or `Bus::fake()`. This is achieved using `UnleashFake`.
[NEW] Variant support. Get the correct variant based on the flag configuration using `Unleash::variant('flag-name', 'default-value');`
[CHANGE] To better support mocking and variants, the package has been updated to use value objects. This should be backwards compatible for the most part, with the implementation fo `ArrayAccess`, except for checks using `is_array()`.
[CHANGE] The Laravel-like `enabled()`, `disabled()`, `get()` and `all()` methods are now the primary, with the standard Unleash methods as aliases
[CHANGE] Tests have been updated to better handle Config mocking, simplifying some of them dramatically.
@dshafik dshafik force-pushed the dshafik/add-variant-support-refactor branch from 98c3006 to 93cfafd Compare July 12, 2022 17:26
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.

None yet

2 participants