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

feat(all): add ruff as default linter and additional clean up #922

Merged
merged 50 commits into from
Dec 27, 2024

Conversation

spencer-tb
Copy link
Collaborator

@spencer-tb spencer-tb commented Oct 29, 2024

🗒️ Description

Adds ruff as our default lint/format/import checker replacing flake8, black & isort.
Extra clean-up items are mentioned below.

From this point onwards we should strictly make sure our code adheres to the best Python standards.

🧑‍💻 .vscode

The settings.recommended.json has be split into 2 files:

  • settings.json: locked config that users of the repo MUST use.
  • .settings.local.recommended.json: settings that users can change, for example python testing extension related.

💥 Removal of tf

It seems relevant for us to deperate tf now. Most repo users are familiar with the uv fill etc approach now.

📜 Ruff Related Docstring Changes

A large portion of the diff comes from changes to the python docstrings, via:

One-line docstring should fit on one lineRuff D200
First line of docstring should be in imperative mood: "Handles all fill CLI flag pre-processing."Ruff D401
First line should end with a periodRuff D400

With a new linter its seems logical to enforce python docstring standards from now on. Note the warnings appear within VSCode and additionally when running tox. This required a lot of manual intervention to update our docstrings to use imperative mood.

🔑 Key Notes & Checks

  • Filling for Cancun with EELS returns the same index.json root hash as upstream/main.
  • Tox passes locally & in CI for tests, framework and docs.
  • Nothing else has been checked by me.

🔗 Related Issues

☑️ Remaining Todos

  • Investigate all new mypy errors when running uv run mypy src/tests locally.
    • Note these don't exist when running mypy within a tox run, its a similar behaviour to main just with more errors.
  • Update src/cli/gentest/source_code_generator.py to use ruff instead of black.
  • Add codespell for spell checking. Note this PR removes fname8.
  • Remove all the noqa: and type: ignore adding appropriate solutions.

✅ Checklist

  • All: Set appropriate labels for the changes.
  • All: Considered squashing commits to improve commit history.
  • All: Added an entry to CHANGELOG.md.
  • All: Considered updating the online docs in the ./docs/ directory.

@spencer-tb spencer-tb added the type:feat type: Feature label Oct 29, 2024
@spencer-tb spencer-tb changed the title feat(all feat(all): add ruff as the default linter for EEST Oct 29, 2024
@spencer-tb spencer-tb changed the title feat(all): add ruff as the default linter for EEST feat(all): add ruff as default linter and additional clean up Oct 29, 2024
@spencer-tb spencer-tb force-pushed the dog-goes-ruffruffruff branch 5 times, most recently from b3f6ae4 to 5e61b48 Compare October 30, 2024 09:29
@marioevz
Copy link
Member

marioevz commented Nov 1, 2024

@spencer-tb could we rebase this to main please? I think it would make it easier to spot issues in the CI if we include the fixes from #905

@spencer-tb spencer-tb self-assigned this Nov 27, 2024
@spencer-tb spencer-tb force-pushed the dog-goes-ruffruffruff branch 2 times, most recently from a32b069 to f80467a Compare December 7, 2024 06:50
@spencer-tb spencer-tb added scope:tests Scope: Changes EL client test cases in `./tests` type:chore Type: Chore scope:fw Scope: Framework (evm|tools|forks|pytest) scope:tooling Scope: Python tools (uv, ruff, tox,...) labels Dec 7, 2024
@spencer-tb spencer-tb force-pushed the dog-goes-ruffruffruff branch from 7349bc7 to ee354f9 Compare December 7, 2024 15:30
Copy link
Collaborator Author

@spencer-tb spencer-tb left a comment

Choose a reason for hiding this comment

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

Some comments:

  1. I have only tested out filling. We get the same index root hash as main. It would be nice for others to test out consume, execute and other clis.
  2. Tox passes tests/framework/docs for me locally and in the CI with the new changes. 😄
  3. I noticed that running mypy locally outside of tox gave me a lot of errors. These errors are not flagged during tox runs. This is a similar behaviour to main, however with the ruff changes there are substantially more. I think this is okay but its worth looking into.

Quoting the remaining todo's here:

  • Investigate all the new mypy error when running uv run mypy src/tests locally.
    • Not these don't exist when running tox locally but its a similar behaviour to main just more errors.
  • Update src/cli/gentest/source_code_generator.py to use ruff instead of black.
  • Add codespell for spell checking.
  • Remove all the noqa: and type: ignore adding an appropriate solution.

src/cli/check_fixtures.py Show resolved Hide resolved
src/cli/gen_index.py Show resolved Hide resolved
@spencer-tb spencer-tb added the scope:docs Scope: Documentation label Dec 7, 2024
.vscode/settings.json Show resolved Hide resolved
.vscode/settings.local.recommended.json Show resolved Hide resolved
pyproject.toml Show resolved Hide resolved
src/cli/eest/commands/clean.py Show resolved Hide resolved
src/cli/gentest/source_code_generator.py Show resolved Hide resolved
src/cli/gentest/test_context_providers.py Show resolved Hide resolved
@spencer-tb spencer-tb marked this pull request as ready for review December 7, 2024 17:07
@danceratopz danceratopz force-pushed the dog-goes-ruffruffruff branch from ee18346 to 824b49f Compare December 10, 2024 11:02
Copy link
Member

@marioevz marioevz left a comment

Choose a reason for hiding this comment

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

Amazing job!

I will rebase, and double-check on the --fix/--no-fix parameters, then merge.

tox.ini Outdated Show resolved Hide resolved
@marioevz marioevz force-pushed the dog-goes-ruffruffruff branch from 824b49f to a8b0a68 Compare December 27, 2024 17:58
@marioevz marioevz merged commit 7e00240 into main Dec 27, 2024
6 checks passed
@marioevz marioevz deleted the dog-goes-ruffruffruff branch December 27, 2024 18:20
codeofcarson pushed a commit to codeofcarson/execution-spec-tests that referenced this pull request Jan 24, 2025
…reum#922)

* feat: use ruff for linting.

* chore: overhaul `.vscode` files.

* chore(ruff): changes to `src/cli`.

* chore(noqa|todo): changes to `src/cli`.

* chore(tf): remove `tf` from the repo.

* chore(ruff|noqa): changes to `src/config`.

* chore(ruff): changes to `src/ethereum_clis`.

* chore(noqa): changes to `src/ethereum_clis`.

* chore(ruff): changes to `src/ethereum_test_base_types`.

* chore: remove `.flake8` config.

* chore(ruff): changes to `src/cli` after rebase.

* chore(ruff): changes to `src/ethereum_clis` after rebase.

* chore(ruff): changes to `src/ethereum_test_base_types` after rebase.

* chore(ruff): changes to `src/ethereum_test_exceptions`.

* chore(ruff): changes to `src/ethereum_execution`.

* chore(ruff): changes to `src/ethereum_test_fixtures`.

* chore(ruff): changes to `src/ethereum_test_forks`.

* chore(ruff): changes to `src/ethereum_test_rpc`.

* chore(ruff): changes to `src/ethereum_test_specs`.

* chore(ruff): changes to `src/ethereum_test_tools`.

* chore(ruff): changes to `src/ethereum_test_types`.

* chore(ruff): changes to `src/ethereum_test_vm`.

* chore(ruff): changes to `src/conftest.py`.

* chore(ruff): changes to `src/pytest_plugins`.

* chore(ruff): changes to `tests/shanghai`.

* chore(ruff): changes to `tests/prague`.

* chore(ruff): changes to `tests/paris`.

* chore(ruff): changes to `tests/osaka`.

* chore(ruff): changes to `tests/istanbul`.

* chore(ruff): changes to `tests/homestead`.

* chore(ruff): changes to `tests/frontier`.

* chore(ruff): changes to `tests/constantinople`.

* chore(ruff): changes to `tests/cancun`.

* chore(ruff): changes to `tests/byzantium/`.

* chore(ruff): changes to `tests/berlin`.

* chore(ruff): changes to `src` to fix filling.

* chore: mypy fixes part 1.

* chore: mypy fixes part 2.

* chore(docs): update changelog.

* chore(docs): update ruff in docs.

* tests: fix eip2537 ruff changes.

* chore: revert uv.lock to 88c1401 for a gentler package upgrade

* chore: only add ruff & remove isort etc to/from deps in uv.lock

* chore: pin ruff to version 0.8.2

* style(lint): fix docstrings in github script

* style(lint): fix python builtin shadowing (in ruff 0.8.2)

This was required after upgrading ruff from 0.7.1 to 0.8.2

* style(lint): fix import block (in ruff 0.8.2)

This was required after upgrading ruff from 0.7.1 to 0.8.2

* fix(tox.ini): Add ruff format checks

* fix(fw): Formatting

* fix(tests): Formatting

---------

Co-authored-by: danceratopz <[email protected]>
Co-authored-by: Mario Vega <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope:docs Scope: Documentation scope:fw Scope: Framework (evm|tools|forks|pytest) scope:tests Scope: Changes EL client test cases in `./tests` scope:tooling Scope: Python tools (uv, ruff, tox,...) type:chore Type: Chore type:feat type: Feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants