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
Add ENABLE_DEBUG_TOOLBAR to settings #3983
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3983 +/- ##
==========================================
+ Coverage 91.22% 91.28% +0.06%
==========================================
Files 267 267
Lines 14443 14534 +91
Branches 1391 1387 -4
==========================================
+ Hits 13176 13268 +92
Misses 897 897
+ Partials 370 369 -1
Continue to review full report at Codecov.
|
Since it's a PR for the debug toolbar, could you take a minute to bump |
@@ -250,7 +250,9 @@ def get_bool_from_env(name, default_value): | |||
'phonenumber_field', | |||
'captcha'] | |||
|
|||
if DEBUG: | |||
|
|||
ENABLE_DEBUG_TOOLBAR = get_bool_from_env('ENABLE_DEBUG_TOOLBAR', False) |
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.
Should we really disable it by default?
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.
If we don't want the next line
if DEBUG and ENABLE_DEBUG_TOOLBAR:
then yes. Otherwise, it will be added on production servers by default and we for sure don't want that.
6efb691
to
682c226
Compare
@NyanKiyoshi lets bump dependencies in another PR |
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.
I strongly believe that DDT time has passed. Both storefront and dashboard use gql api + js front, and in this configuration DDT provides no feedback. Silk is in this area a much better choice.
Keeping that in mind I think that change default to have it disabled is a good choice.
I want to merge this change because...
#3978
Pull Request Checklist