chore: tidy up makefile (DEV-1166) #223
Conversation
| @@ -19,7 +19,6 @@ jobs: | |||
| python-version: 3.9 | |||
| - name: Install dependencies | |||
| run: | | |||
| make upgrade-dist-tools | |||
There was a problem hiding this comment.
I deleted this command. It is better to include the dist-tools as dev-dependencies (in the dev-section of Pipfile). This way, they get installed when make install-requirements is executed. We don't need a separate command just to install the dist-tools.
| # Make targets for dsp-tools | ||
| ################################# | ||
|
|
||
| .PHONY: clone-dsp-repo |
There was a problem hiding this comment.
We don't need an extra command to clone the DSP-API. I included this in the dsp-stack command.
Makefile
Outdated
| .PHONY: stack-down | ||
| stack-down: ## stop dsp-stack and remove the cloned dsp-api repository | ||
| $(MAKE) -C .tmp/dsp-stack stack-down-delete-volumes | ||
| @sudo rm -rf .tmp |
There was a problem hiding this comment.
For developer's machines, we need a command to stop the dsp-stack, otherwise it continues running for a long time, consuming resources.
The command to remove the .tmp folder needs a sudo because otherwise, it produces an error on GitHub (this error here: https://github.com/dasch-swiss/dsp-tools/runs/8204985409?check_suite_focus=true). The sudo wouln't be necessary for local machines, but for GitHub, it is necessary. If there are better solutions than sudo, I'm open for correction.
There was a problem hiding this comment.
Is it necessary to remove the tmp folder? Why not just leave it?
There was a problem hiding this comment.
make dsp-stack is the only place in the entire repo where the tmp folder is created. So it makes sense to me to delete it in the stack-down command, instead of the make clean command.
| .PHONY: docs-publish | ||
| docs-publish: ## build and publish docs to GitHub Pages | ||
| mkdocs gh-deploy | ||
| mkdocs serve --dev-addr=0.0.0.0:7979 |
There was a problem hiding this comment.
docs-publishis not used anymore, because the docs are only on docs.dasch.swiss.- for
mkdocs serve, I need another port, because the standard port is occupied on my machine.
| pytest test/e2e/ | ||
|
|
||
| .PHONY: test-unittests | ||
| test-unittests: ## run unit tests | ||
| pytest test/unittests/ | ||
|
|
||
| .PHONY: local-tmp |
There was a problem hiding this comment.
This is now included at the only place where it is necessary: before cloning DSP-API.
|
|
||
| .PHONY: help | ||
| help: ## show this help | ||
| @awk 'BEGIN {FS = ":.*?## "} /^[a-zA-Z_-]+:.*?## / {printf "\033[36m%-30s\033[0m %s\n", $$1, $$2}' $(MAKEFILE_LIST) | sort | ||
|
|
||
| .PHONY: run |
There was a problem hiding this comment.
This command is not necessary, IMHO.
| @@ -32,6 +32,8 @@ wheel = "*" | |||
| pipenv-setup = "*" | |||
| pytest = "*" | |||
| types-requests = "*" | |||
| twine = "*" | |||
There was a problem hiding this comment.
These two dependencies were "hidden" until now in docs/requirements.txt and in make upgrade-dist-tools. I think they belong here.
There was a problem hiding this comment.
I don't think we use mkdocstrins at all - it's a Mkdocs plugin that generates documentation based on the docstrings in the source code... and we don't have that, as far as I know. So probably this could be removed, right?
There was a problem hiding this comment.
I removed it, and GitHub CI runs through without errors. So I assume we really don't need it.
| @@ -32,6 +32,8 @@ wheel = "*" | |||
| pipenv-setup = "*" | |||
| pytest = "*" | |||
| types-requests = "*" | |||
| twine = "*" | |||
There was a problem hiding this comment.
I don't think we use mkdocstrins at all - it's a Mkdocs plugin that generates documentation based on the docstrings in the source code... and we don't have that, as far as I know. So probably this could be removed, right?
reformat setup.py
Makefile
Outdated
| .PHONY: stack-down | ||
| stack-down: ## stop dsp-stack and remove the cloned dsp-api repository | ||
| $(MAKE) -C .tmp/dsp-stack stack-down-delete-volumes | ||
| @sudo rm -rf .tmp |
There was a problem hiding this comment.
Is it necessary to remove the tmp folder? Why not just leave it?
autoformat setup.py with autopep8
makefile: improve creation and removal of .tmp
resolves DEV-1166