Adds method disabled, inverse of enabled

[kinduff]

Hello there!

First of all, a lot of thanks to @jnunemaker, the maintainers and the community for this wonderful gem. We use it a lot @dmstk (domestika.org and we can't be more happy.

One thing we've noticed with heavy use (around 20-25 feature flags and more to come) is that we usually end up doing this in order to have the inverse of a gate:

return if counter == 1 and !Flipper.enabled?(:feature)

This is okay, but it's not as explicit as we would like.
v
The proposed change adds a new method in the Gate class in order to have the inverse of the method, like so:

return if counter == 1 and Flipper.disabled?(:feature)

We can of course do this downstream, but having it as part of the gem would be golden.

I did not see the necessity of adding tests for this since the enabled? method is being tested in all the gates, but let me know if you want me do so.

Thank you!

[kinduff]

@jnunemaker any chance this can get a look? if it's something you're not looking to have, happy to close it up

[bkeepers]

@kinduff could you explain more about the real world use case? I know you gave an abstract example, but my brain is having a hard time imagining a use case where unless wouldn’t fit the bill.

It doesn’t seem like it would be hard to add ‘disabled?’, I’m just curious to see a concrete example.

[jnunemaker]

@kinduff I could have sworn I responded to this. But I must have in my head and not on github because here we are. 😆 Sorry about that!

Something similar to this (maybe the same thing) has come up before (previous response). Can you take a look at that and let me know if you have any thoughts? Tried to link directly to my response but you might need to scan up or down a bit for more context if what I'm saying doesn't make sense.

tldr: The primary issue is flipper is about enabling things and letting them through gates, not disabling things. Making flipper work as is for disabling is easy for storage (the adapters) but makes the super simple api of flipper (a selling point IMO) a lot more complicated to understand.

I really want to make this work somehow. But it is not an easy, simple change. If anyone comes up with a simple solution or even just a bunch of responses that make sense to the matrix I listed above, I'd be happy to implement it or merge a PR. I'll probably sit down with @bkeepers soon and talk through this one. We'll probably be able to come up with something that I couldn't on my own.

[jnunemaker]

Closing to keep things tidy but I really do want to keep talking about this if you have more thoughts.

[jnunemaker]

I actually wonder if instead of calling it disabled? (which feels right but could fudge things down the road), we call it not_enabled?. Flipper.not_enabled?(...) isn't as pretty but is more descriptive of what is actually happening. We are checking to see if something is disabled. We are only checking to see if it is NOT enabled. Subtle but different.

[kinduff]

Hey @jnunemaker! Totally missed the notifications around this.

I actually like the idea a lot more, and makes sense given the context you posted, as well as to keep things simple. The word disabled can lead to incorrect assumptions of which way the stuff needs to be toggled.

Regarding the use case @bkeepers asked about, I cannot think of a clearer example as the one I posted. I think it really depends on the implementation. We usually need to add features that are both toggleable and backward compatible, and a method like this instead of using a bang operator can clean up our code a lot.

Happy to contribute and add a PR to add the method, or even continue with the discussion.

[jnunemaker]

@kinduff just wrote up more here #514. I really like being able to disable specific actors as well. That said, it might not quite aline with what you are attempting.

I think what @bkeepers was trying to ask was why not just do this:

return if counter == 1
return unless Flipper.enabled?(feature)

I often do a set of guards at the beginning of a method like that rather than checking them all on one line. Views are probably the only place where more than one conditional on a line happens for me.