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

Update AS::Notifications::Instrumenter#instrument #35705

Merged
merged 3 commits into from Mar 22, 2019

Conversation

@alimi
Copy link
Contributor

@alimi alimi commented Mar 22, 2019

Summary

ActiveSupport::Notifications provides a powerful API for instrumentation by using a publish-subscribe pattern. We could leverage this for more use cases by opening up ActiveSupport::Notifications messaging for things other than instrumentation. To make that happen, I opened this PR to update ActiveSupport::Notifications::Instrumenter#instrument so passing a block is optional. This will let us use #instrument for general messaging in addition to instrumentation.

/cc @tenderlove @eileencodes

  * Update #instrument to make passing a block optional. This will let users
    leverage #instrument for messaging in addition to instrumentation.
Copy link
Member

@eileencodes eileencodes left a comment

Can you update the docs and add a changelog?

@@ -20,7 +20,7 @@ def instrument(name, payload = {})
# some of the listeners might have state
listeners_state = start name, payload
begin
yield payload
yield payload if block_given?
Copy link
Member

@eileencodes eileencodes Mar 22, 2019

Can you update the docs above this method?

alimi added 2 commits Mar 22, 2019
with change to ActiveSupport::Notifications::Instrumenter#instrument
@rails-bot rails-bot bot added the docs label Mar 22, 2019
@alimi
Copy link
Contributor Author

@alimi alimi commented Mar 22, 2019

I updated the changelog and docs in aaf89cd and updated the guides in 7237c7b.

@eileencodes eileencodes merged commit 75f19b5 into rails:master Mar 22, 2019
3 checks passed
@eileencodes eileencodes added this to the 6.0.0 milestone Mar 22, 2019
@alimi alimi mentioned this pull request Mar 22, 2019
@alimi alimi deleted the instrumenter-block-optional branch Mar 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants