-
Notifications
You must be signed in to change notification settings - Fork 16.3k
Bugfix/remove global from breeze #58866
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
Bugfix/remove global from breeze #58866
Conversation
amoghrajesh
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.
@jscheffl I think this one can be done much cleaner using function attributes: https://www.pythonmorsels.com/storing-attributes-on-functions-in-python/
d80adca to
46ff0be
Compare
Okay, good idea. At least for "ci_group" made this as feedback. For the others I think it is not making it better. Unfortunately the WDYT? |
|
Looks great! we can improve it in the future if we find a better way. |
* Remove global from breeze modules * Remove global from breeze modules * Review feedback, use function attributes (cherry picked from commit fc70635) Co-authored-by: Jens Scheffler <95105677+jscheffl@users.noreply.github.com>
| global _in_ci_group | ||
| if _in_ci_group or skip_group_output(): | ||
| if getattr(ci_group, "__in_ci_group__", False) or skip_group_output(): |
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.
Perfect @jscheffl! This is what I was suggesting
* Remove global from breeze modules * Remove global from breeze modules * Review feedback, use function attributes
* Remove global from breeze modules * Remove global from breeze modules * Review feedback, use function attributes
* Remove global from breeze modules * Remove global from breeze modules * Review feedback, use function attributes
Another small increment to remove global statements for PR #58116
This removes all leftover global statements in breeze. In most cases
globalwas mis-used for some minor flags.globalis evil. Also in breeze :-D