search

ScatterHQ / machinist

A library for constructing finite state machines
Apache License 2.0
57 stars 12 forks source link

Receive either plain or rich input #7 #17

Closed cyli closed 10 years ago

cyli commented 10 years ago

Eh, it was basically done. I'm just putting this up here - if we'd rather go with 2 parameters as @itamarst suggested, we could modify this or just close this one and open a new one.

Note that this is based on #15, so if that gets merged this diff will be shorter. Update: #15 has been merged, this has been rebased.

Fixes #7

cyli commented 10 years ago

It looks like the travis tests are breaking against the latest version of eliot. Looks like #24, when merged and this is rebased should fix it.

itamarst commented 10 years ago

24 has been merged.

cyli commented 10 years ago

Rebased, ready for review

exarkun commented 10 years ago

Thanks! Overall I like this change. Please address the inline comments - in particular, those related to testing - then re-open. Thanks again.

exarkun commented 10 years ago

Okay apparently I can't close pull requests when using waffle. :cactus:

cyli commented 10 years ago

Thanks for the thorough review, @exarkun! I think I've addressed the comments, but I had two questions (which are now in collapsed comments):

https://github.com/hybridcluster/machinist/pull/17#discussion_r12936111 and https://github.com/hybridcluster/machinist/pull/17#discussion_r12905486

exarkun commented 10 years ago

Thanks! This looks great now. Merging.