-
Notifications
You must be signed in to change notification settings - Fork 299
docs: improve onboarding #1775
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
docs: improve onboarding #1775
Conversation
CodSpeed Performance ReportMerging #1775 will not alter performanceComparing Summary
|
acf04d5
to
90fae44
Compare
Onboarding was not up to date now that `uv` is really supported everywhere. - `make install` now only relies on `uv` - `make help` is a new target to describe all targets with their meaning - README setup section is way clearer
please review |
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.
Thanks, this looks good with just one thing about the uv
<> pre-commit
interaction which I'm unsure about.
Makefile
Outdated
uv sync --frozen --group all | ||
uv pip install -v -e . | ||
pre-commit install | ||
uv pip install pre-commit |
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 this get wiped by future uv sync
commands? Should we use uv tool install pre-commit
instead? That will affect global state, though, so we might have to go back to just recommending it? 🤔
README.md
Outdated
With rust and python 3.9+ installed, compiling pydantic-core should be possible with roughly the following: | ||
You'll need: | ||
1. **[Rust](https://rustup.rs/)** - Rust stable (or nightly for coverage) | ||
2. **[Python 3.9+](https://www.python.org/downloads/)** - Python 3.9 or later |
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.
Should we note here that uv
can handle the install automatically?
@davidhewitt thank you for the review! I took you remarks into account in two last commits |
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.
Thanks!
Onboarding was not up to date now that
uv
is really supported everywhere.make install
now only relies onuv
make help
is a new target to describe all targets with their meaningSelected Reviewer: @sydney-runkle