-
Notifications
You must be signed in to change notification settings - Fork 4
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
Add makefile with common commands #23
base: main
Are you sure you want to change the base?
Conversation
|
||
.DEFAULT_GOAL := venv/bin/activate | ||
|
||
venv/bin/activate: requirements-dev.txt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe just venv
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's not how makefile works, "functions" below check for existance of venv/bin/activate file if it doesn't exist then makefile will run this "function"
|
||
- name: Cache pre-commit | ||
uses: actions/cache@v3 | ||
with: | ||
path: ~/.cache/pre-commit | ||
key: pre-commit-3|${{ env.pythonLocation }}|${{ hashFiles('.pre-commit-config.yaml') }} | ||
|
||
- name: Install pre-commit | ||
run: pip3 install pre-commit | ||
- name: Install venv and pre-commit | ||
run: make |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i guess we shouldn't be so brutal :D we want to use cached version to make this step much quicker and avoid setting up venv from scratch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem is that we can't run make functions without creating a virtual environment and sourcing it first. Because of this, I'm not sure if it's worth continuing to work on the Makefile. Since we can't use it everywhere, it would require maintaining code in two places—both the Makefile and the ci.yml.
venv/bin/activate: requirements-dev.txt | ||
python$(PYTHON_VERSION) -m venv venv | ||
. venv/bin/activate | ||
pip install --upgrade pip |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am getting following output:
~/Documents/Projects/ds-template/{{ cookiecutter.repo_name }}$ make
python3.11 -m venv venv
. venv/bin/activate
pip install --upgrade pip
make: pip: No such file or directory
make: *** [Makefile:9: venv/bin/activate] Error 127
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the error still occur if you run make after generating the test project? Can you check if pip is configured correctly on your machine?
|
||
.DEFAULT_GOAL := venv/bin/activate | ||
|
||
venv/bin/activate: requirements-dev.txt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need this requirements-dev.txt
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
makefile will check if requirments file changed, if yes then venv will be rebuild :)
- . setup_dev_env.sh | ||
- pip install build twine bump2version | ||
- make | ||
- pip install build twine bump2version #??? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess it may be removed indeed
before_script: | ||
- make |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as we use $PRECOMMIT_IMAGE, it is not necessary to setup venv before
Changelog:
TODO: