chore[build]: simplify onboarding#4886
Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6c52924092
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| setup( | ||
| name="vyper", | ||
| use_scm_version={ | ||
| "local_scheme": _local_version, | ||
| "local_scheme": _local_version_suffix, | ||
| "version_scheme": _global_version, | ||
| "write_to": "vyper/version.py", |
There was a problem hiding this comment.
setuptools_scm for the setup.py install path
The documented pip path still goes through make init / make.cmd init, which invoke python setup.py install, but this file no longer bootstraps setuptools_scm. In a fresh venv with only setuptools installed, python setup.py --version now warns that use_scm_version is unknown and reports 0.0.0, so contributors following the README get an incorrectly versioned compiler unless they manually install setuptools_scm first.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Following the setup steps in the readme for both uv and pip leads to the correct version
(checked on a mktemp --directory)
A version of 0.0.0 is most likely due to forgetting to fetch the tags
📊 Bytecode Size Changes (venom)No changes detected. Full bytecode sizes
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4886 +/- ##
==========================================
+ Coverage 91.93% 91.98% +0.05%
==========================================
Files 186 186
Lines 27567 27564 -3
Branches 4816 4816
==========================================
+ Hits 25343 25355 +12
+ Misses 1509 1493 -16
- Partials 715 716 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| python: "3.11" | ||
| jobs: | ||
| pre_create_environment: | ||
| - asdf plugin add uv |
There was a problem hiding this comment.
readthedocs env has asdf?
There was a problem hiding this comment.
It appears to do, Claude did that and it worked (the current readthedocs failure was caused by the outdated main, not these changes)
There was a problem hiding this comment.
i just don't understand why we need it, is it so that readthedocs runs uv? but why do we need that
There was a problem hiding this comment.
It's to be able to remove the requirements-docs.txt file
(as suggested by its own TODO)
There might be a way to make it work with pip, but IIRC it also involved using asdf
There was a problem hiding this comment.
It also makes it easier to test the docs locally
There was a problem hiding this comment.
Moved to pip in the end
|
looks like readthedocs build is failing |
This reverts commit f376a25.
|
|
||
| ```bash | ||
| make dev-init | ||
| ./quicktest.sh -m "not fuzzing" |
There was a problem hiding this comment.
why did you remove this?
There was a problem hiding this comment.
I moved it to the installation instructions, since it only concerns pip
| 1. Clone this repo and `cd` into it | ||
| 2. Run `git fetch --tags git@github.com:vyperlang/vyper.git` to get the tags necessary for version inference | ||
| 3. Run `uv sync` | ||
| 4. Run `source .venv/bin/activate` to activate the virtual environment |
There was a problem hiding this comment.
does uv venv or something do this
There was a problem hiding this comment.
uv venv create a venv, but does not activate it (probably for safety reasons ?)
And it's performed as part of uv sync
For automatically activating the venv there are things like direnv, which I highly recommend, but was not sure if it should be added to the readme, since it's not required
There was a problem hiding this comment.
Seems like a good tutorial ?
https://www.ashleyconnor.co.uk/til/2025-03-01-how-to-automatically-activate-venv-with-direnv
I use a more convoluted way with nix shells, so I've not tried it tho
|
|
||
| ## Developing (working on the compiler) | ||
| 1. Clone this repo and `cd` into it | ||
| 2. Run `git fetch --tags git@github.com:vyperlang/vyper.git` to get the tags necessary for version inference |
There was a problem hiding this comment.
does the compiler not work without this step?
There was a problem hiding this comment.
It does run, but fails if the file has a version pragma, and as a result the tests do not pass
| [metadata] | ||
| license_files = LICENSE | ||
|
|
||
| # TODO: Does anyone use `python setup.py test` ? |
There was a problem hiding this comment.
no, that should probably be removed
There was a problem hiding this comment.
why do we now need setup.py in addition to pyproject?
There was a problem hiding this comment.
As requested, I moved the metadata back to setup.py, while the rest (dependencies and their settings) are in pyproject.toml
It's possible to also have almost everything in pyproject.toml
(the only thing I was not able to move was the git-tag-based version logic, since we do something non-standard)
And it's also not possible to not have pyproject.toml
- black requires it, which is why it existed before this PR
- IIRC uv requires it
There was a problem hiding this comment.
As an additional argument to having metadata in pyproject.toml, the PyPA recommends it:
On the other hand, configuring setuptools with a setup.py file is still fully supported, although it is recommended to use the modern [project] table in pyproject.toml (or setup.cfg) whenever possible
Source: https://packaging.python.org/en/latest/guides/tool-recommendations/#build-backends
2 participants
What I did
How I did it
How to verify it
Commit message
Description for the changelog
Cute Animal Picture