-
-
Notifications
You must be signed in to change notification settings - Fork 719
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
Fix bugsnag warning #11733
Fix bugsnag warning #11733
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Yasir, thanks for looking into this. I'm wondering if it would work to set a random string
for the key, eg:
config.api_key = "anything"
Would you be able to try this? Then we won't need to expose the actual api key.
If so, I'm not sure if this change here is needed (it seems to do the same thing as the removed line config.notify_release_stages = %w(production staging)
No luck setting a random string as api, same warning is displayed. |
Hi Yasir, thanks for your work on this. While pondering about it, I had an idea: if we don't use Bugsnag in dev, then we don't need to load the gem at all. I wasn't sure if it would work so went ahead to try it out (#11782). |
I think you're completely right, thanks for pointing this out. So the only ways to hide the errors are:
I'd rather not add a dependency on an account, so I suggest changing the log level, would you like to do this? |
@dacook Sure I can work on this in the weekend. |
0a7a1f5
to
dcac87e
Compare
@dacook I have suppressed the log level for bugsnag warning in dev env, now the warning has disappeared in dev. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great! Just a couple if minor changes to make it excellent. 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good one 👍
Co-authored-by: Maikel <maikel@email.org.au>
Co-authored-by: Maikel <maikel@email.org.au>
2b3f8fd
to
703ef8a
Compare
@mkllnk Updated as per your suggestions |
I just tested it, It works as expected. Merging. |
What? Why?
What should we test?
text_field
totex_field
and gotohttp://localhost:3000/admin/reports/customers
. And check the logs.In Master: - Bugsnag warning is printed
In feature branch: Bugsnag warning should not be printed
Release notes
Changelog Category (reviewers may add a label for the release notes):
The title of the pull request will be included in the release notes.
Dependencies
Documentation updates