Skip to content
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

[WIP] Taskfile support #28

Closed
wants to merge 5 commits into from
Closed

[WIP] Taskfile support #28

wants to merge 5 commits into from

Conversation

HeCorr
Copy link

@HeCorr HeCorr commented May 7, 2023

This PR implements support for Task, a cross-platform Make alternative written in Go.

See #2.

HeCorr added 2 commits May 6, 2023 23:05
it accepts an optional `task` argument which makes it run task commands instead of make commands (the default if no argument is supplied).

since I'm on Windows I had to add a parameter to .gitattributes so I could save watch.sh with LF line endings, otherwise it would fail to be executed in Linux.
`BINARY_NAME` is also utilized as the Docker image name (line 26), so update the comment to reflect that.
@HeCorr
Copy link
Author

HeCorr commented May 7, 2023

Unfortunately Task doesn't handle interrupt signals very well so I'm not sure if watch.sh is working properly:

image

I would have to test that with a custom executable but I don't think that's a priority, unless there's a chance of database corruption in the case of an unexpected shutdown. @thomiceli Please let me know if that's the case.

@HeCorr
Copy link
Author

HeCorr commented May 7, 2023

@thomiceli is this echo on line 33 correct?

opengist/Makefile

Lines 32 to 34 in 2951cb0

watch_backend:
@echo "Building Opengist binary..."
DEV=1 ./node_modules/.bin/nodemon --watch '**/*' -e html,yml,go,js --signal SIGTERM --exec 'go run . --config config.yml'

If it's not please let me know what it's supposed to say instead and I'll fix it.

HeCorr added 3 commits May 6, 2023 23:39
note: `task watch` doesn't currently work on Windows due to the dependency on `watch.sh`. the other tasks should work just fine, however I have not tested that.
Task can handle running tasks in parallel, so use that instead of relying on the `watch.sh` script which has shown to be incompatible with Task's interrupt signaling.

also update `watch` task's description.
@HeCorr
Copy link
Author

HeCorr commented May 7, 2023

As per c898285, the watch.sh script is no longer executed by Task, which has worked... better, but still not perfect:

image

At least now the backend seems to be properly shutting down, based on the still waiting for 1 sub-process to finish message.

It seems the watch_frontend failure error above is due to Vite returning exit code 1 when exiting with Ctrl+C/SIGINT. I did try to make it return 0 instead but that somehow caused the backend task to not exit properly... go figure.

@thomiceli thomiceli linked an issue May 7, 2023 that may be closed by this pull request
@thomiceli
Copy link
Owner

thomiceli commented May 7, 2023

@thomiceli is this echo on line 33 correct?

opengist/Makefile

Lines 32 to 34 in 2951cb0

watch_backend:
@echo "Building Opengist binary..."
DEV=1 ./node_modules/.bin/nodemon --watch '**/*' -e html,yml,go,js --signal SIGTERM --exec 'go run . --config config.yml'

If it's not please let me know what it's supposed to say instead and I'll fix it.

Indeed it's not correct, it should be something like "Running Opengist and watching for changes..."; same idea for watch_frontend

EDIT : did not see you already changed it, all good!

@HeCorr
Copy link
Author

HeCorr commented May 8, 2023

EDIT : did not see you already changed it, all good!

I didn't yet, what I did was add descriptions but those don't get logged. I'll be fixing it soon

@HeCorr
Copy link
Author

HeCorr commented Jun 20, 2023

once again, my apologies for the inactivity. I've been quite busy.

@thomiceli
Copy link
Owner

Closing for now; maybe we can use a Taskfile in the future

@thomiceli thomiceli closed this Jan 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Taskfile support
2 participants