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
allow threshold argument to passes_threshold?
to enable re-use in subclasses
#102
Conversation
@@ -55,7 +55,7 @@ def enabled_for?(feature_recipient) | |||
return false if feature_recipient.nil? | |||
return false if blacklisted?(feature_recipient) | |||
return true if whitelisted?(feature_recipient) | |||
passes_threshold?(feature_recipient) | |||
passes_threshold?(feature_recipient, self.deployment_percentage || 0) |
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.
no need for self.
@@ -81,8 +81,7 @@ def self.last_updated_at | |||
|
|||
protected | |||
|
|||
def passes_threshold?(feature_recipient) | |||
threshold = self.deployment_percentage || 0 | |||
def passes_threshold?(feature_recipient, threshold) |
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.
a test that shows this usage ... or some docs why this is here would be nice
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.
it's not a public interface, so a test feels inappropriate. What are you concretely asking for?
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.
... at least docs would be nice ... # used by external library, do not modify interface
also means we are using a protected method ... making it public might be more obvious ... but either way is fine
👍 |
👍 thanks for humoring me |
👍 |
@grosser can you release v2.2.3? I merged and tagged and everything but don't have the perms to release |
done, next time you can do it yourself ;)
…On Mon, Dec 5, 2016 at 5:07 PM, Ben Osheroff ***@***.***> wrote:
@grosser <https://github.com/grosser> can you release v2.2.3? I merged
and tagged and everything but don't have the perms to release
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#102 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAAsZ8k0_oqp9DcQmASDmdYKGmy_mZR5ks5rFLVkgaJpZM4LE0JB>
.
|
@ggrossman @grosser