-
Notifications
You must be signed in to change notification settings - Fork 16.5k
[sip-15] Enabling SIP-15 by default #9017
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
[sip-15] Enabling SIP-15 by default #9017
Conversation
3dbb2b7 to
2c5f733
Compare
superset/config.py
Outdated
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 add in a grace period date here? Maybe 2020-03-31 or something like that?
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.
@etr2460 I feel like indefinite is preferred to having a hard date, it would be troublesome if someone deployed this change and weren't aware that the grace period had already finished and thus this feature would serve as a hard transition.
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.
@mistercrunch @willbarrett do we want to take a passive (no end date) or aggressive (end date specified) approach?
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'm not speaking for @mistercrunch , but I would recommend the aggressive approach. That would provide us a milestone for when the flag can be removed.
UPDATING.md
Outdated
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.
sp nit: for specifying how long chart
2c5f733 to
dd835cc
Compare
dd835cc to
61935f2
Compare
Codecov Report
@@ Coverage Diff @@
## master #9017 +/- ##
=======================================
Coverage 59.15% 59.15%
=======================================
Files 367 367
Lines 11686 11686
Branches 2866 2866
=======================================
Hits 6913 6913
Misses 4594 4594
Partials 179 179Continue to review full report at Codecov.
|
etr2460
left a comment
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 lgtm regardless of if the grace period is defaulted or not, but ping @mistercrunch and @willbarrett for their thoughts
|
I think for now the default should have SIP-15 enabled which will mean by default an infinite grace period. At a later date we can decide when we want to deprecate the old logic and thus hard-code a transition date. |
CATEGORY
Choose one
SUMMARY
This PR enables SIP-15 by default.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TEST PLAN
ADDITIONAL INFORMATION
REVIEWERS
to: @etr2460 @robdiciuccio @willbarrett