Skip to content

Conversation

@codekitchen
Copy link
Contributor

Implements #30

This is based heavily on the raven-python code, with some tweaks to make it more ruby-ish.

Let me know if any changes are needed. I'm not sure if you use a specific ruby styleguide.

@dcramer
Copy link
Member

dcramer commented Dec 11, 2014

I haven't looked at the code, but huge +1 for getting this into more clients :)

@codekitchen
Copy link
Contributor Author

Yeah, it's a blocker to us using sentry in production, we require a circuit breaker or something similar for all 3rd party services.

@nateberkopec
Copy link
Contributor

Hype! You've got some require issues and I haven't looked at the code yet either, but this is great.

@codekitchen
Copy link
Contributor Author

ah, looks like timecop (used only in specs) doesn't support ruby 1.8. i'll see what i can do.

@codekitchen
Copy link
Contributor Author

hm, that fixes ruby 1.8 but it looks like there's a few failures on the ruby-head builds. are those generally pretty stable? i'll see about installing ruby head locally to try and repro.

@nateberkopec
Copy link
Contributor

@codekitchen They have been fairly stable until now.

@nateberkopec
Copy link
Contributor

We should really allow-failure on ruby-head though :| Take a quick look but don't kill yourself.

@codekitchen
Copy link
Contributor Author

OK, I got the latest nightly snapshot installed. I can reproduce the 3 failures locally, but I can also reproduce them on raven-ruby master -- so I think it must be caused by a recent change to ruby head, not related to this change. How would you prefer to proceed? Mark them as allow failure?

@nateberkopec
Copy link
Contributor

Yeah, go ahead and move ruby-head into allow-fail in a separate commit.

@codekitchen
Copy link
Contributor Author

sure thing, allow-fail added in https://github.com/getsentry/raven-ruby/pull/268

@nateberkopec
Copy link
Contributor

Merged, go ahead and rebase.

This is based on raven-python's implementation, with some tweaks for
ruby idioms.

With this commit, if we don't send, we only log the event id. I'd like
to extend this to log the entire event, along with also logging the
entire event when a sending failure occurs.
Using timecop to freeze at the *precise* moment where the condition
changes can cause problems due to rounding error.
@codekitchen
Copy link
Contributor Author

sorry about the spam, i kept forgetting to test ruby 1.8 before pushing

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove the parentheses from any method calls with no arguments.

@nateberkopec
Copy link
Contributor

@codekitchen One minor style quibble and then this is mergable.

@codekitchen
Copy link
Contributor Author

sure thing, done

@nateberkopec
Copy link
Contributor

Thanks! Appreciate the contribution.

nateberkopec added a commit that referenced this pull request Dec 12, 2014
@nateberkopec nateberkopec merged commit 43bb3c6 into getsentry:master Dec 12, 2014
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