Feat/add uv and semantic release#30
Conversation
juliette0704
requested review from
BruceWouaigne
and removed request for
BruceWouaigne
cjumel
left a comment
cjumel
left a comment
There was a problem hiding this comment.
I think it'd be nice to split this PR in two, one focusing on switching the project to uv, the other one for the introduction of the semantic release (which I'm not familiar with so this will be more difficult to review for me 😬)
Besides the comments I made, I have two additional remarks:
- adding a small, concise description in the PR is nice in cases somebody who doesn't have the context of this work (like me 😇) has to look at this
- if we're going to use something like the conventional commit parser, then it can be nice to follow this convention, typically in your PR title (e.g. using
chore: switch to uv), if you plan to squash and merge your PR - can you change the project structure to use the
srclayout? (you can check or other repositories, or use theuv init --libcommand to generate an example)
|
|
||
| ```bash | ||
| pip install linkup-sdk | ||
| uv pip install linkup-sdk |
There was a problem hiding this comment.
This command shouldn't be changed, this describes how to install the package from the PyPI index, so it can still be using pip (which is still the standard from the community globally)
|
|
||
| [dependency-groups] | ||
| dev = [ | ||
| "distlib==0.3.9", |
There was a problem hiding this comment.
I don't think you need more than what was previously in the requirements-dev.txt in this section
| [project] | ||
| authors = [] | ||
| dependencies = [ | ||
| "cohere>=5.14.0", |
There was a problem hiding this comment.
This dependencies (the main ones) are defined in the setup.py file already (in install_required), so make sure they are the same; globally the setup.py should be replaced by the new set up in the pyproject.toml and can be removed afterwards
| with: | ||
| python-version: ${{ matrix.python-version }} | ||
|
|
||
| - name: Install uv |
There was a problem hiding this comment.
This will also setup python, so you can remove the set up python step I think (check the documentation maybe but this is what we did in some of our other repositories)
| install: | ||
| @echo "Installing local package..." | ||
| pip install . | ||
| uv sync | ||
| uv run pre-commit install | ||
| install-dev: | ||
| @echo "Installing local package in developper mode..." | ||
| pip install -e . | ||
| pip install -r requirements-dev.txt | ||
| pre-commit install | ||
| uv pip install -e ".[dev]" | ||
| uv run pre-commit install |
There was a problem hiding this comment.
I don't think we need both make install and make install-dev, maybe we can keep only make install-dev (since that the ones that we really care about), with uv sync (which does include the development deps) and the pre-commit setup, for when doing local development
2 participants
Description
Checklist
make install-devandmake testREADME.mdif my changes affected itlinkup/_version.pyif I plan on creating a new release