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

infra: migrate to uv #43

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

infra: migrate to uv #43

wants to merge 12 commits into from

Conversation

JacobCoffee
Copy link
Member

Description

  • Swap to uv

@JacobCoffee
Copy link
Member Author

@JacobCoffee
Copy link
Member Author

why is preview so bad? idk.

@JacobCoffee
Copy link
Member Author

wrt uv migration this is ready

Copy link

github-actions bot commented Oct 8, 2024

Documentation preview will be available shortly at https://litestar-org.github.io/type-lens-docs-preview/43


.PHONY: install
install: clean ## Install the project, dependencies, and pre-commit for local development
@if ! $(PDM) --version > /dev/null; then echo '=> Installing PDM'; $(MAKE) install-pdm; fi
@if ! $(uv) --version > /dev/null; then echo '=> Installing uv'; $(MAKE) install-uv; fi
@if [ "$(VENV_EXISTS)" ]; then echo "=> Removing existing virtual environment"; fi
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm not sure what the rationale behind all the conditionality with venvs and tooling and whatever is, but i feel like it'd be overall easier if it...didn't exist? like to me the Makefile's only purpose is to define how the project-specific tooling ought to operate. If you're not using (uv in this case) the tooling, then you'd just not use the makefile?

The reason i bring it up here being there'd be no reason to have detroy/clean commands at all really.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

im not sure if this was from trying to support multiple things at once (pip, poetry) or not but it can die

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason i bring it up here being there'd be no reason to have detroy/clean commands at all really.

What's the workflow you are thinking here?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just that you almost dont even need a make install. by default uv run pytest will sync ahead of time. so i'm just not sure what cleaning out the venv would actually do.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm mostly coming from my very simple makefiles from e.g. cappa which are just ways to ensure the command i run locally is exactly what's getting run in CI.

that, and it's really easy to see the total set of linters/formatters being applied to the project. by comparison, i've found the makefile setup (and pre-commit in particular) to be a confusing impediment

"covdefaults",
"pytest",
"pytest-cov",
]

# TODO: needs migrating. Poe?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do these need to exist? each basically has a corresponding make command

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it can go away for now. i think the idea was to shuffle things into pyproject and lighten the Make targets

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for Makefile entry.

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.

3 participants