pavlos / gen_fsm

Elixir wrapper around OTP's gen_fsm
Other
40 stars 3 forks source link

adding typespecs #4

Closed gausby closed 8 years ago

gausby commented 8 years ago

I have added some typespecs.

They are not complete yet, I miss handle_event/3, handle_sync_event/4 handle_info/3, and code_change/4.

I have looked at the implementation for the Elixir core gen_server wrapper, and I've tried to follow the Erlang documentation on the different specs for the functions. I am not 100% sure they are correct.

It's a start. You can let this pull request linger and I'll get back to adding specs for the remaining callbacks. Perhaps I need to reorder some of the specs and types, and use the types for state_name and state_data consistently.

pavlos commented 8 years ago

wow, this is great. Thank you so much! I will let the PR sit until you want me to merge it

gausby commented 8 years ago

Thanks. Let me know if you see something that is wrong :)

gausby commented 8 years ago

I've added the remaining callbacks, and I've added the turnstile example as a test because it test the sync calls—the commits has been squashed into one, so it shouldn't mess up the history too much.

Merge it if you find it good.

pavlos commented 8 years ago

@gausby this is awesome, please check out the comments I left and let me know what you think.

gausby commented 8 years ago

Thanks for the feedback. I've replied to most of them!

gausby commented 8 years ago

I've fixed the server vs fsm_ref, I went with fsm_ref :)

I am going to squash the commit and force push it to the branch to keep the history clean.

gausby commented 8 years ago

I guess the only outstanding issue is wether or not to include state name and event name with the default callback implementation of event and sync event callbacks. I would still argue that the crash report will contain this information, so we don't need to pass it to the return tuple. {:stop, :unexpected_event, state_data} will quite simply do in my opinion.

That is how I have my current gen_fsm yasnippet setup in my editor, as seen here: https://github.com/gausby/mg-elixir-snippets/blob/master/snippets/elixir-mode/behaviour-gen-fsm :)

pavlos commented 8 years ago

let's leave out the state/event name, and we can always add it back in if it becomes a problem

pavlos commented 8 years ago

merged!