Add operation environment variable #287
Conversation
There was a problem hiding this comment.
It looks like a good first approach.
However I was hoping for a deeper refactoring of how tasks work. My idea was to run tasks always: pre-copy, post-copy, pre-update, post-update. The updatediff case is a special one that I'd like to review deeper, because tasks need to run there in the temporary subproject generated to obtain the diff.
Also I'm wondering if having a complex _tasks configuration is needed, given any script can just operate based on env variables.
What are your thoughts?
| @@ -290,6 +301,9 @@ def update_diff(conf: ConfigData) -> None: | |||
| ) | |||
| } | |||
| ) | |||
| # At this point we'll run tasks in the current project: | |||
| # copy_local() needs to pass "update" as operation | |||
| os.environ["OPERATION"] = "update" | |||
There was a problem hiding this comment.
This is too much, you're altering the environment variables for all the process.
You better do:
with local.env(OPERATION='update'):
...Docs: https://plumbum.readthedocs.io/en/latest/local_machine.html#environment
There was a problem hiding this comment.
We could also consider passing an operation argument to copy_local, defaulting to copy, as it's the only place we use os.environ.get("OPERATION").
yajo
added
the
hacktoberfest-accepted
|
I've open #288 after thinking more about this. |
|
I'll close this, it's not ready and I won't be able to work on it any time soon. |
|
Thanks, I'll keep it in mind when fixing #240. |
First pass at #240.