-
-
Notifications
You must be signed in to change notification settings - Fork 18
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
Duplicated decorators during an edge case of named and unnamed decorators #1325
Conversation
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.
Note that the CI is failing. See the linter and the comment of Dawid on tmg.
In the end allowed either all named or all unnamed decorators |
Unit Tests Summary 1 files 70 suites 1h 11m 13s ⏱️ Results for commit ca761ee. ♻️ This comment has been updated with latest results. |
Unit Test Performance Difference
Results for commit 0b8c3a6 ♻️ This comment has been updated with latest results. |
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 like the decision to only allow named or unnamed decorators, as the code is the same for tmg and there you provided an updated code example I replied there. The messages the users get and the code do not always match. See insightsengineering/teal.modules.general#835 (review)
…assert_decorators, and let it pass through normalize_decorators
…tors (#835) Companion to insightsengineering/teal.modules.clinical#1325 --------- Signed-off-by: Marcin <133694481+m7pr@users.noreply.github.com> Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
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.
Same behavior as on TMG. Good work
Fixes #1316
Companion in tmg insightsengineering/teal.modules.general#835
There was an issue where decorators got duplicated when you passed a combination of named an unnamed decorators into the module. This module has 2 objects, so if you pass 1 unnamed decorators and one named, there should be only 3 decorators visible (1 unnamed passed to all 2, and one named pass to one named).
On feature branch
On main
Code