docs: update CONTRIBUTING.md for uv and ruff
#1396
Conversation
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## forks/osaka #1396 +/- ##
============================================
Coverage 94.94% 94.94%
============================================
Files 583 583
Lines 34665 34665
Branches 3070 3070
============================================
Hits 32914 32914
Misses 1198 1198
Partials 553 553
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
CONTRIBUTING.md
Outdated
| $ mypy src # Verifies type annotations. | ||
| $ pytest -n 4 # Runs tests parallelly. | ||
| $ pytest -m "not slow" # Runs tests which execute quickly. | ||
| tox -e static # Run linting and type checking. |
There was a problem hiding this comment.
Not sure if we still need this after the commands above.
There was a problem hiding this comment.
The aim was have a one-liner to copy-paste for those unfamiliar with tox (and don't know the -e flag).
Co-authored-by: Guruprasad Kamath <[email protected]>
danceratopz
requested review from
Carsons-Eels and
gurukamath
and removed request for
gurukamath
| * `geth` installed and present in `$PATH` | ||
| - [`uv`](https://docs.astral.sh/uv/) package manager, | ||
| - Python 3.11.x, | ||
| - [PyPy](https://www.pypy.org/) [7.3.19](https://downloads.python.org/pypy/) or later (typically only required by execution-specs maintainers), |
There was a problem hiding this comment.
Anyone running tox locally will have to have PyPy installed.
| - [`uv`](https://docs.astral.sh/uv/) package manager, | ||
| - Python 3.11.x, | ||
| - [PyPy](https://www.pypy.org/) [7.3.19](https://downloads.python.org/pypy/) or later (typically only required by execution-specs maintainers), | ||
| - `geth` installed and present in `$PATH` (only required by users who would like to sync and verify mainnet blocks using EELS; not required for typical specification or test development). |
There was a problem hiding this comment.
geth is required for execution-specs/tests/json_infra/test_ethash_general.py, but that's marked as a slow test so is never run. Syncing mainnet works with ~any client.
| It is recommended to run the `static` checks locally before pushing changes to a pull request: | ||
|
|
||
| `execution-specs` depends on a submodule that contains common tests that are run across all clients, so we need to clone the repo with the --recursive flag. Example: | ||
| ```bash | ||
| $ git clone --recursive https://github.com/ethereum/execution-specs.git | ||
| uvx tox -e static # Run linting and type checking. | ||
| ``` | ||
|
|
||
| Or, if you've already cloned the repository, you can fetch the submodules with: | ||
| All environments can be executed using (very slow, not recommended, even in parallel): | ||
|
|
||
| ```bash | ||
| $ git submodule update --init --recursive | ||
| uvx tox | ||
| ``` |
There was a problem hiding this comment.
I don't think we should tell people not to run the full tox locally. I'd rather the recommendation be something like run tox -e static often during development, and tox once before opening your pull request.
| Environments can be executed in parallel with (still very slow): | ||
|
|
||
| ```bash | ||
| $ tox | ||
| uvx tox run-parallel | ||
| ``` |
There was a problem hiding this comment.
Is this a safe recommendation? The PyPy environment OOM'd in CI with 32GiB of memory.
| uv run ethereum-spec-fill tests/eest --clean --fork Osaka -k eip7939 | ||
| ``` | ||
|
|
||
| #### Spec Unit Tests |
There was a problem hiding this comment.
| #### Spec Unit Tests | |
| #### Debugging Tests |
I might be missing the point of the subsection?
| ### New Fork Tool | ||
| ----------------- | ||
|
|
||
| This tool can be used to create the base code for a new fork by using the existing code from a given fork. | ||
|
|
||
| The command takes 4 arguments, 2 of which are optional | ||
| * from_fork: The fork name from which the code is to be duplicated. Eg. - "Tangerine Whistle" | ||
| * to_fork: The fork name of the new fork Eg - "Spurious Dragon" | ||
| * from_test (Optional): Name of the from fork within the test fixtures in case it is different from fork name. Eg. - "EIP150" | ||
| * to_test (Optional): Name of the to fork within the test fixtures in case it is different from fork name Eg - "EIP158" | ||
|
|
||
| - from_fork: The fork name from which the code is to be duplicated. Eg. - "Tangerine Whistle" | ||
| - to_fork: The fork name of the new fork Eg - "Spurious Dragon" | ||
| - from_test (Optional): Name of the from fork within the test fixtures in case it is different from fork name. Eg. - "EIP150" | ||
| - to_test (Optional): Name of the to fork within the test fixtures in case it is different from fork name Eg - "EIP158" | ||
|
|
||
| As an example, if one wants to create baseline code for the `Spurious Dragon` fork from the `Tangerine Whistle` one | ||
|
|
||
| ```bash | ||
| ethereum-spec-new-fork --from_fork="Tangerine Whistle" --to_fork="Spurious Dragon" --from_test=EIP150 --to_test=EIP158 | ||
| uv run ethereum-spec-new-fork --from_fork="Tangerine Whistle" --to_fork="Spurious Dragon" --from_test=EIP150 --to_test=EIP158 | ||
| ``` | ||
|
|
||
| The following will have to however, be updated manually | ||
|
|
||
| 1. The fork number and `MAINNET_FORK_BLOCK` in `__init__.py`. If you are proposing a new EIP, please set `MAINNET_FORK_BLOCK` to `None`. | ||
| 2. Any absolute package imports from other forks eg. in `trie.py` | ||
| 3. Package names under `setup.cfg` | ||
| 4. Add the new fork to the `monkey_patch()` function in `src/ethereum_optimized/__init__.py` | ||
| 5. Adjust the underline in `fork/__init__.py` |
There was a problem hiding this comment.
This'll all get replaced when #1372 is merged.
What was wrong?
CONTRIBUTING.mdstill referred toblack,isortand didn't prefix entrypoints withuv; still referred to git submodules..gitmoduleswas empty/unused.How was it fixed?
Updated
CONTRIBUTING.mdand removed.gitmodules.Cute Animal Picture