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

Reset notifier before first fork #8

Conversation

jonah-williams
Copy link
Contributor

Fixes cases where a Notifier may have been created prior to the Rollbar initializer calling Rollbar.configure. Previously Resque would continue to use this prematurely created Notifier and never use the Configuration defined by the initializer. See rollbar/rollbar-gem#183

Jonah Williams added 2 commits November 24, 2014 10:46
The previous version of rollbar_spec.rb would pass even if `lib/resque/rollbar.rb`'s `before_first_fork` block was empty since it used the default Rollbar configuration which is synchronous.
Fixes cases where a `Notifier` may have been created prior to the Rollbar initializer calling `Rollbar.configure`. Previously Resque would continue to use this prematurely created `Notifier` and never use the `Configuration` defined by the initializer. See rollbar/rollbar-gem#183
@jonah-williams jonah-williams force-pushed the reset_notifier_before_first_fork branch 3 times, most recently from 8fae944 to d5f3091 Compare November 24, 2014 19:51
@jonah-williams
Copy link
Contributor Author

Is maintaining Rollbar 0.8.x compatibility still important for this gem? If so how would you like to handle spec cases and Rollbar method calls which are only available in the Rollbar 1.x release?

@jonah-williams
Copy link
Contributor Author

The call to reset_notifer! might not be needed depending on how rollbar/rollbar-gem#183 is resolved.

I think the spec here is still a false positive, at least when running against Rollbar 1.x so it might be worth addressing in any case.

@dmeremyanin
Copy link
Owner

Is maintaining Rollbar 0.8.x compatibility still important for this gem? If so how would you like to handle spec cases and Rollbar method calls which are only available in the Rollbar 1.x release?

I think we should release a new version for >= 1.2.0 only.

@jonah-williams
Copy link
Contributor Author

@dimko sure thing, how does this look now?

dmeremyanin added a commit that referenced this pull request Dec 4, 2014
@dmeremyanin dmeremyanin merged commit 7197921 into dmeremyanin:master Dec 4, 2014
@dmeremyanin
Copy link
Owner

Just merged, thanks for the contribution and sorry for the delay!

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.

3 participants