-
Notifications
You must be signed in to change notification settings - Fork 37
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
Enable copy_from and after conditions with multiple arguments. #120
Enable copy_from and after conditions with multiple arguments. #120
Conversation
So that we can use a list of operation functions instead of a single operaiton function as argument.
@mikemhenry @vyasr Any chance you could review this PR soon? Or should I try to find someone else? |
I can review, I started going through all the issues/PRs associated with me yesterday and will get around to this one sometime today. |
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.
Nice feature. One note, depending on the sequence of the workflow you could end up evaluating conditions multiple times if they're conditions for multiple conditions passed to these decorators. However, not sure that there's a whole lot to be done about that, and it shouldn't matter except for massive projects.
I agree and I've created issue #125 to keep track of 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.
I like this generalization. It makes sense and is clean.
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.
Is it possible to add a unit test for this feature?
@mikemhenry I've added a unit test to specifically test this feature. |
LGTM 🎉 |
Thank's for the help @vyasr @mikemhenry @b-butler ! |
Description
Enable the provision of multiple operation functions as argument to
pre.after()
andpre/post.copy_from()
Motivation and Context
When generating operation functions programmatically, it is much more convenient to store those in a list and then we can use those lists as argument to the condition functions. Otherwise we cannot use the decorator syntax.
Minimal example:
Without this feature we would need to awkwardly call
Project.pre.after
as a function manually:Types of Changes
1The change breaks (or has the potential to break) existing functionality.
Checklist:
If necessary: