Add option to run dependencies asynchronously#106
Conversation
|
This looks great! Thanks for the contribution. Would you kindly add some documentation on this feature before I accept the PR https://github.com/joerdav/xc/tree/main/doc/content/task-syntax Thanks! |
|
Documented 👌🏾 |
|
Brilliant, thanks for that. Now that parrallel tasks are possible do you think we should do something about the STDOUT of the tasks? Since they may get mangled now. And a question on |
Well.... the ideal experience may be to do like
Well, I could see people wanting both ways, but I honestly don't know which would be better. I almost want to suggest have both behaviours as options, but it starts to feel even more complex. The other idea I had was to allow the user group the async task with brackets. Something like this. requires: build-assets, [watch-assets, watch-server], cleanupWhich will run the build assets first, then run the watchers async, before the cleanup. But again... more complexity. |
|
I think we can maybe tackle the logging of async tasks later. For me I think the sensible default would be for the async tasks to all complete before continuing. I think after that change I would be happy to accept the change provided all the workflows pass! I believe the grouped behaviour is currently achievable once we merge this branch too (even if it's a bit messy): ## run
requires: build-assets, watch, cleanup
## watch
requires: watch-assets, watch-server
runDeps: async
## watch-server
...
## watch-assets
...
## build-assets
...
## cleanup
... |
|
Made the change to not exit on the first error. |
|
I've added log prefixes to all scripts run by I understand that this would technically be a breaking change, but I think it is allowed since |
|
I've also fixed a bug with I'm not sure if that's the best solution, so please take a look |
The prefixes are padded with spaces when the task has dependencies
e2e3a54 to
a26d23e
Compare
|
It looks like the lint step failed, are you able to fix these? It looks like there was also a large drop in test coverage, I think some tests around the log file you created should be acceptable. |
Fixed linting and added tests |
|
Can you take another look @joerdav |
|
I've also added an "Interactive" attribute with some documentation of where it can be useful. |
Fixes #105