getsentry / sentry-ruby

Sentry SDK for Ruby
https://sentry.io/for/ruby
MIT License
933 stars 494 forks source link

Refactor Sentry.close #2207

Closed st0012 closed 10 months ago

st0012 commented 10 months ago

When reviewing #2206, I spotted 2 things that could be improved:

skip-changelog

codecov[bot] commented 10 months ago

Codecov Report

Merging #2207 (9fb2a55) into master (dd2d617) will increase coverage by 0.00%. The diff coverage is 100.00%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #2207 +/- ## ======================================= Coverage 97.36% 97.36% ======================================= Files 101 101 Lines 3798 3799 +1 ======================================= + Hits 3698 3699 +1 Misses 100 100 ``` | [Components](https://app.codecov.io/gh/getsentry/sentry-ruby/pull/2207/components?src=pr&el=components&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry) | Coverage Δ | | |---|---|---| | [sentry-ruby](https://app.codecov.io/gh/getsentry/sentry-ruby/pull/2207/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry) | `98.07% <100.00%> (+<0.01%)` | :arrow_up: | | [sentry-rails](https://app.codecov.io/gh/getsentry/sentry-ruby/pull/2207/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry) | `94.98% <ø> (ø)` | | | [sentry-sidekiq](https://app.codecov.io/gh/getsentry/sentry-ruby/pull/2207/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry) | `94.53% <ø> (ø)` | | | [sentry-resque](https://app.codecov.io/gh/getsentry/sentry-ruby/pull/2207/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry) | `92.06% <ø> (ø)` | | | [sentry-delayed_job](https://app.codecov.io/gh/getsentry/sentry-ruby/pull/2207/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry) | `94.44% <ø> (ø)` | | | [sentry-opentelemetry](https://app.codecov.io/gh/getsentry/sentry-ruby/pull/2207/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry) | `100.00% <ø> (ø)` | | | [Files](https://app.codecov.io/gh/getsentry/sentry-ruby/pull/2207?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry) | Coverage Δ | | |---|---|---| | [sentry-ruby/lib/sentry-ruby.rb](https://app.codecov.io/gh/getsentry/sentry-ruby/pull/2207?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry#diff-c2VudHJ5LXJ1YnkvbGliL3NlbnRyeS1ydWJ5LnJi) | `96.44% <100.00%> (+0.01%)` | :arrow_up: |
st0012 commented 10 months ago

ah you mean send them in the main thread

Yeah. I don't see a reason to send it async at that stage while waiting to shutdown the worker right away.