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

More detailed status report upon configuration #309

Merged
merged 1 commit into from
Mar 12, 2015

Conversation

faber
Copy link
Contributor

@faber faber commented Mar 12, 2015

I was getting thrown off by the "ready to catch errors" message that was popping up even in environments where it was configured not to send. My use case is that I have a Raven.configure block in my code all the time, but whether or not I actually enable capture is controlled the presence/absence of the SENTRY_DSN env var. Since calling Raven.capture triggers the message, it came up every time regardless of whether I was actually capturing errors.

(I know this is a somewhat superficial PR, sorry!)

Two points open for debate (besides whether or not this PR is desired/worthwhile at all) are the actual text used to indicate that errors will not be sent, and whether or not to alias report_status with report_ready, for backwards compatibility purposes. Technically report_ready is part of the public API, so even tho its doubtful that anyone was using it directly, better safe than sorry I suppose. Another option is just leaving it named report_ready, but that seems marginally less correct.

Cheers!

@dcramer
Copy link
Member

dcramer commented Mar 12, 2015

+1 from my end -- anything that makes it more obvious whats going on, without being intrusive is generally useful

@nateberkopec
Copy link
Contributor

Thanks!

nateberkopec added a commit that referenced this pull request Mar 12, 2015
More detailed status report upon configuration
@nateberkopec nateberkopec merged commit 8e6f8db into getsentry:master Mar 12, 2015
@faber faber deleted the report-status branch March 12, 2015 19:58
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