Skip to content

Conversation

PrettyWood
Copy link
Collaborator

@PrettyWood PrettyWood commented Aug 3, 2025

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

Selected Reviewer: @sydney-runkle

Copy link

codspeed-hq bot commented Aug 3, 2025

CodSpeed Performance Report

Merging #1775 will not alter performance

Comparing onboarding (ca770ad) with main (57e4411)

Summary

✅ 159 untouched benchmarks

@PrettyWood PrettyWood force-pushed the onboarding branch 3 times, most recently from acf04d5 to 90fae44 Compare August 3, 2025 16:39
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
@PrettyWood
Copy link
Collaborator Author

please review

Copy link
Contributor

@davidhewitt davidhewitt left a 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
Copy link
Contributor

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
Copy link
Contributor

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?

@PrettyWood
Copy link
Collaborator Author

@davidhewitt thank you for the review! I took you remarks into account in two last commits

Copy link
Contributor

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

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

Thanks!

@davidhewitt davidhewitt merged commit 17cbe98 into main Aug 5, 2025
31 checks passed
@davidhewitt davidhewitt deleted the onboarding branch August 5, 2025 09:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants