Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for event triggered transitions #63

Closed
wants to merge 10 commits into from

Conversation

iurimatias
Copy link
Contributor

implements #31 and #56 , similar functionality to aasm

@carlosparamio
Copy link

Nice job. 👍

@markquezada
Copy link

This looks great. Are there any issues/limitations with this approach?

@joefiorini
Copy link

This looks incredibly useful. I'm looking at using Statesman on a new project after having using Transitions in the past. Having events makes it easier to wrap transitions in domain language. I may use this fork until it gets pulled in. Thanks @iurimatias!

@markquezada
Copy link

@mrappleton any thoughts on this?

@appleton
Copy link
Contributor

appleton commented Jul 1, 2014

Whilst I'm not sure I would use a feature like this, there does seem to be a lot of support for it, so I'm happy to go ahead and merge it once a few comments are tidied up. Thanks for the contribution!

@@ -189,6 +217,19 @@ def transition_to!(new_state, metadata = nil)
true
end

def trigger!(event_name, metadata = nil)
transitions = self.class.events.fetch(event_name) do
raise Statesman::TransitionFailedError
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to have a message explaining the error here.

@appleton
Copy link
Contributor

appleton commented Jul 1, 2014

Generally looks good! Happy to merge if you could address the comments.

@iurimatias
Copy link
Contributor Author

@markquezada Yes, a feature not implemented (yet) is callbacks for the events.

@appleton appleton closed this in 8c8ef44 Jul 29, 2014
@appleton
Copy link
Contributor

Thanks for the contribution, this will be in v0.8.0 which I'm just about to push to rubygems.

appleton added a commit that referenced this pull request Jul 29, 2014
@markquezada
Copy link

@mrappleton 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants