-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
deprecate using None
for code_execution_config
#1506
deprecate using None
for code_execution_config
#1506
Conversation
None
for code_execution_config
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1506 +/- ##
===========================================
+ Coverage 33.08% 53.59% +20.51%
===========================================
Files 42 42
Lines 5048 5051 +3
Branches 1156 1224 +68
===========================================
+ Hits 1670 2707 +1037
+ Misses 3250 2112 -1138
- Partials 128 232 +104
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
LGTM
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.
LGTM
Why are these changes needed?
As a user I would expect providing a value of
None
forcode_execution_config
would disable the feature. It is instead used as a sentinel for the "default" case. This change is the first step in rectifying this confusion.Related issue number
N/A
Checks