Fix subscriptions not being unsubscribed.

[tgxworld]

See https://github.com/rails/rails/blob/master/activesupport/lib/active_support/notifications.rb

[tgxworld]

@senny Please review. Came across it while working on actionpack. Thanks :)

[tgxworld]

I just noticed the failing test, will fix soon

[tgxworld]

@senny Fixed :)

[senny]

@tgxworld the docs mention:

  # The +subscribe+ method returns a subscriber object:
  #
  #   subscriber = ActiveSupport::Notifications.subscribe("render") do |*args|
  #     ...
  #   end
  #
  # To prevent that block from being called anymore, just unsubscribe passing
  # that reference:
  #
  #   ActiveSupport::Notifications.unsubscribe(subscriber)

Can we rename the instance variables to _subscriber?

[tgxworld]

@senny Thanks for the review :) I've made the changes.

Just to point out, I guess the changes here are fine since there was a conflicting use of @subscriber? :)

[senny]

@tgxworld yea that's fine. Just went through the docs and saw that we usually call it subscriber. Good to keep that wording. Thanks for the update.

[senny]

the fact that one subscription was added but forgotten in the removal process is a smell. Let's go one step further and refactor this to:

@_subscribers = []
# ...
@_subscribers << ActiveSupport::Notifications.subscribe("A")
@_subscribers << ActiveSupport::Notifications.subscribe("B")
@_subscribers << ActiveSupport::Notifications.subscribe("C")

# loop over @_subscribers to remove them.

This should be easier to keep in sync.

[tgxworld]

👍

[tgxworld]

@senny Updated per your comments :)

[senny]

@tgxworld looking good 💛