-
Notifications
You must be signed in to change notification settings - Fork 37
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
Default result always override result when it set as true #22
Conversation
99c64c2
to
c4dbb74
Compare
Pull Request Test Coverage Report for Build 174
💛 - Coveralls |
0c443ee
to
01fc69e
Compare
01fc69e
to
c63ea62
Compare
…ck as default strategy behaviour
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your PR! This was a nice catch!
It looks very good, but I have some suggestions on what you can do so we can have it merged.
Appreciate that you took the time to write the tests covering the issue and all other use cases!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
After test the result combinations with default result, it does not work as expected when default value is true. It will always override combined result as true regardless the combined result of toggle is enabled and strategy result.
https://github.com/Unleash/unleash-client-ruby/blob/master/lib/unleash/feature_toggle.rb#L74..L81
The modification will be - when strategy result already computed and toggle is enabled, it may respect that combined result instead of default result.