Raise an error if expected keys are unused

[bschmeck]

This change will cause LightService to raise an error if an action doesn't use one or more of the expects keys. If there are keys that are possibly unused (e.g. as a result of a conditional,) they can be marked as maybe using a new macro.

The motivation for adding this is to help ensure that expects blocks are accurate, which will help code discovery, grepping, etc.

Possibly this should be optional/configurable behavior?

[adomokos]

Thank you for the detailed response!

I do like the concept of :maybe, this PR will be merged. I also see the value of providing a higher level check of :expects: it's not enough for it to be in the context as a key, the user has to interact with it in the action.

I am not a fan of placing :maybe on the same level with :expects and :promisesas it adds another layer of complexity that will be hard to grok for the user.

I like your 3rd suggestion the best as well, as it signals the keys you defined there are special type of :expects. I wonder if we could write this for a single item :maybe:

class MakesTeaMaybeWithMilkAction
  extend LightService::Action
  expects :tea, :use_milk, :maybe => :milk
  promises :milk_tea
end

And this if there are multiple :maybe items:

class MakesTeaMaybeWithMilkAction
  extend LightService::Action
  expects :tea, :use_milk, :maybe => [:milk, :honey]
  promises :milk_tea
end

We will need to add detailed examples in the specs where it's obvious how the expects is changing. I am happy to help with that as well. 😄

[bschmeck]

I've updated the code so that specifying maybe keys happens as a kwarg to the expects declaration. The kwarg can be a single key maybe: milk or an array of keys maybe: [:milk, :tea].

The implementation doesn't prevent you from re-declaring a key. E.g. this is legal:

expects :milk, :maybe => :milk

and results in the key being considered a maybe.

[adomokos]

This looks great, thank you for your work on it!

Since this change will have breaking changes, I'll merge it into the 1.0rc1 branch. I'd also like to eliminate orchestrators and move the reduce_if etc. macros to organizers. That will be a nice 1.0 release. I'll add an issue for that soon.

[adomokos]

I like to start my development on LS (or any other app) with a failing feature spec, that would describe the desired behavior. I did not see new feature specs for this change.

I'd document:

• What happens when you use only maybe keys
• When you use expected keys with maybes
• Only expected keys and no maybes.

I usually point to those specs from the documentation. I am happy to add those myself.

[bschmeck]

Sorry for the delay. I finally had a chance to add some tests in spec/acceptance. Let me know if those are close to what you're looking for.

[adomokos]

Thank you!!