flapjack / flapjack-diner

Consumer of the Flapjack API
http://flapjack.io/
MIT License
17 stars 10 forks source link

Unclear documentation for multiple check_ids on create_scheduled_maintenances_checks #50

Open Sarah-E-Greene opened 9 years ago

Sarah-E-Greene commented 9 years ago

Flapjack::Diner.create_test_notifications_checks(CHECK_ID(S), [TEST_NOTIFICATION, ...])

Is this supposed to be:

Flapjack::Diner.create_test_notifications_checks('foo:http', 'bar:http', [TEST_NOTIFICATION, ...])

or

Flapjack::Diner.create_test_notifications_checks( { 'foo:http', 'bar:http' } , [TEST_NOTIFICATION, ...])

or something else entirely?

The README should be updated to clarify this.

ghost commented 9 years ago

The first is recommended -- I think ['foo:http', 'bar:http'] as a single argument would also work. This code needs better tests written, I can't update the README until I verify the behaviour is actually as I remember it to be. :smile:

Sarah-E-Greene commented 9 years ago

@ali-graham What's your opinion on integrating the flapjack-client tool into flapjack diner, and writing a test suite to cover the lot?

ghost commented 9 years ago

Sounds good -- I think we want to preserve the 'library-style' access for people who'd prefer that, but baking in CLI access as well gives people the best of both worlds. I think it definitely belongs here rather than shipping with the Flapjack gem itself, which longer-term should really only be needed by people wanting to run server-side stuff.

jessereynolds commented 9 years ago

:+1: