Conversation
env_prototype/core.py
Outdated
| return None | ||
|
|
||
|
|
||
| def execute(executable, args=None, environment=None, cwd=None): |
There was a problem hiding this comment.
What's the reason for refactoring this to execute. It's the same method as in Avalon pipeline currently, but there it's called launch? Any reason why the original was a bad name?
|
Still not too sure about having the |
|
I think we can rename |
|
Tested it for Maya and Fusion deadline submission, had to add |
|
I do not think that we should care about familiarity of others when using this. We could have chosen Let's tackle it from a pure logical standpoint without taking in account the OS in which it is used.
Now based on pure grammar: we already KNOW the object, we pass it on to the function. Taking that logic in account both I am entirely in favor to use |
|
No, The name which comes from the fact that the same executable name can be on your path, e.g. python.exe in Python3 and Python2. You use Again, subtle difference. I'm fine with |
|
Any objection to merging this? |
acre/launch.py
Outdated
| import os | ||
| import pprint | ||
|
|
||
| from . import discover, build, merge, locate, launch |
There was a problem hiding this comment.
Is this importing itself? Isn't this a naming conflict? :) Interesting that this is actually allowed.
|
Other than above note this is fine for me - it's a prototype. Like mentioned in our discussions I think the API can still simplify by just having the By the way, don't you think |
|
Made some changes to the API, I would like to have a review on the I think that there is still some room for improvements |
Part of issue FUS-32
whichandexecutefunction to core