-
Notifications
You must be signed in to change notification settings - Fork 109
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
Graduate script annotations #1287
Conversation
* _flag_enabled blocks treated as `True` * `os.environ(...) is None` blocks treated as `False` * `os.environ(...) is not None` blocks treated as `True` Signed-off-by: Elliot Gunton <[email protected]>
Signed-off-by: Elliot Gunton <[email protected]>
Signed-off-by: Elliot Gunton <[email protected]>
Signed-off-by: Elliot Gunton <[email protected]>
Signed-off-by: Elliot Gunton <[email protected]>
Signed-off-by: Elliot Gunton <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1287 +/- ##
=======================================
- Coverage 86.2% 86.1% -0.1%
=======================================
Files 60 60
Lines 4102 4079 -23
Branches 657 648 -9
=======================================
- Hits 3538 3516 -22
Misses 392 392
+ Partials 172 171 -1 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Elliot Gunton <[email protected]>
if script_env: | ||
script.env = script_env |
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.
Isn't this a behaviour change? We used to append to script.env
if it existed, but now we overwrite it.
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.
In the case of user script environment variables, yes. Will fix
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.
Thank you for spotting! #1289
Pull Request Checklist
script_annotations
experimental feature #1286Description of PR
Graduate script annotations
experimental_features["script_annotations"]
flaghera__script_annotations
environment variable to scripts, so no longer need to check it in the runnerutil.py
codeWith this change, the default code path becomes the "script annotations" path, so we always check for
Annotated
, rather than using simple kwarg checks, so we are able to remove the_get_parameters_from_callable
function.