Add a package_preprocess_mode (#609) #651
Conversation
|
I'd like to mention that I feel like Also, please don't merge this for now. I've written tests but I'd like to see if it fits at a bigger scale and see what I can do with it at RodeoFX to confirm the idea and behavior. |
src/rez/developer_package.py
Outdated
| if preprocess_func: | ||
| print_info("Applying preprocess from package.py") | ||
| def _get_package_level(): | ||
| print_info("Applying preprocess from package.py") |
There was a problem hiding this comment.
Message printed too early? I would also say '...from package definition' rather than package.py (you can configure rez to use a different filename).
src/rez/developer_package.py
Outdated
| if not preprocess_func or not isfunction(preprocess_func): | ||
| print_error("Function '%s' not found" % package_preprocess_function) | ||
| return None | ||
|
|
||
| print_info("Applying preprocess function %s" % package_preprocess_function) |
There was a problem hiding this comment.
Message printed too early? Would also mention 'applying global preprocess function'
| ] | ||
|
|
||
| def preprocess(this, data): | ||
| print 'Local preprocess' |
|
Cheers! Only some minor issues that I can see. RE the setting name - I wouldn't worry about this much, I think I'll wait til you're confident after more testing, just let me know. Thx |
| continue | ||
|
|
||
| level = "global" if preprocessor == global_preprocess else "local" | ||
| print_info("Applying {0} preprocess function".format(level)) |
There was a problem hiding this comment.
@nerdvegas I changed the message a little bit while fixing the messages being printed too early. I tried to make the message consistent across the local and global process. If you don't want to introduce the "global" term here, let me know and I'll try to rephrase.
|
Looks good JC. Just clarifying that this is ready to merge now, you said you were doing more testing? |
|
Hey @nerdvegas sorry for taking so long on this. I'm currently in a big rush at work and it takes me quite a lot of my time. I've booked myself some time tomorrow to test this out a little bit more. |
|
@nerdvegas I tested more in depth and it's all good to go. You can merge this PR! Really sorry for the delay on this one. |
This PR implements #609 , which is make it possible to have both a global and local preprocess function run when building a package.
That means it's now possible to have a global preprocess that applies to all packages, and also have specialized preprocesses per packages if need be. This is made possible by a new configuration called
package_preprocess_mode. It accepts 3 values:after,beforeandoverride.The default value is
override, which mean we have the exact same behavior as before: If you definedpackage_preprocess_functionand you also have apreprocessfunction in a package, only the package'spreprocessfunction is called.afterandbeforeare what I call the additive modes. They allow you to choose to execute a package'spreprocessfunction before or after the global preprocess.