Handle cases where caller forgets question mark

[moonboots]

@schleyfox @randyzhao

I've accidentally called launch instead of launch?. It's confusing because it doesn't raise an exception and always returns false.

[BMorearty]

@moonboots I think it behaves this way already. See the specs. https://github.com/airbnb/trebuchet/blob/master/spec/trebuchet_spec.rb#L39-L45

[moonboots]

@BMorearty I'm not sure how the rspec should helpers are monkey patched, but I think they're affecting the results. Here's rails console snippet from our dev environment with the nil return value:

irb(main):016:0> Trebuchet.aim('testing', :everyone)                                                                  
=> #<Trebuchet::Feature name: "testing", strategy: launched to everyone>
irb(main):017:0> Trebuchet.current.launch('testing')
=> nil
irb(main):018:0> Trebuchet.current.launch?('testing')
=> true

[schleyfox]

I'd strongly prefer this to throw an ArgumentError or something if a block is not provided.

[BMorearty]

I was originally going to say the same thing @schleyfox said...until I saw that the specs specifically say it's ok to call it without a block.

@moonboots your irb example is certainly a compelling reason to make a change. Since the specs seem to be incorrectly passing, let's change it to throw an ArgumentError if the block is not provided. And change those specs.

This would be a breaking change but unfortunately the version is 0.10.0 so we can't use semantic versioning to indicate that it's a breaking change. It might be time to move this to 1.0. I found some callers who use launch without a block where they meant to call launch?.

[BMorearty]

P.S. If you're not willing to make the breaking change right now, you'd at least need to make sure the tests are correct before merging this change. By "correct" I mean they should fail without this change and succeed with it.