remove the aasm dependency #14
Conversation
|
Thanks! I'm a little busy right now, but will take a closer look this weekend. |
|
Any news on this? |
|
(Y) |
|
just rebased, @wsargent could you have a look? |
|
@yurivm okay, thanks! |
|
👍 |
|
@yurivm can you please fix the conflicts? |
| @@ -59,8 +59,7 @@ Gem::Specification.new do |s| | |||
| s.extra_rdoc_files = ["History.txt", "README.txt"] | |||
| s.rdoc_options = ["--main", "README.txt", "--charset=UTF-8"] | |||
|
|
|||
| s.add_runtime_dependency "aasm" | |||
|
|
|||
| s.add_development_dependency "aasm" | |||
There was a problem hiding this comment.
I prefer to only include dev deps in the gemspec when required to run tests. I'm thinking the tests should be run with asm and without, and the 'with' can be achieved using a BUNDLE_GEMFILE and eval_gemfile, or use the 'appraisals' gem
| aasm.state :open | ||
|
|
||
| aasm.state :closed, :enter => :reset_failure_count | ||
| def trip |
There was a problem hiding this comment.
I imagine this is equivalent to what is generated by
aasm.event :trip do
transitions :to => :open, :from => [:closed, :half_open]
eend
| transitions :to => :closed, :from => [:open, :half_open] | ||
| # if AASM is required elsewhere it will call this method to get current state | ||
| def aasm(_) | ||
| OpenStruct.new(current_state: @state) |
should probably be its own commit |
|
It's been almost 3 years since I opened the original PR, so at this point I can't really remember what I did there or why - closing in favor of #22 |
Hi,
first of all, thanks for the gem! I've noticed there was an issue open about AASM being the single dependency and how nice it would be to have none.
So I went ahead and removed it - the circuit_breaker doesn't have a lot of states or complex transitions, nor does it require a lot of callbacks or saving state via ActiveRecord.
I've kept the backwards compatibility by raising AASM::InvalidTransition if that exception is defined, but the check is cached, not sure if that affects performance.
Other than that, rspec was throwing a lot of warnings about the .should syntax so I went ahead and updated those to expect(..).
Thanks for taking a look!