feat(ci,tooling): improve static check developer experience#2099
feat(ci,tooling): improve static check developer experience#2099danceratopz wants to merge 12 commits into
Conversation
Split the monolithic [testenv:static] into individual check environments for finer-grained control and parallel execution: - spellcheck: codespell. - lint: ruff check. - format: ruff format --check. - typecheck: mypy. - spec-lint: ethereum-spec-lint. - lockcheck: uv lock --check. - actionlint: GitHub Actions workflow linting. Add fast-checks aggregate alias and keep static for backwards compat. Ref: ethereum#2041
Add scripts/fast_checks.py that wraps each static check with actionable fix hints on failure: - Prints clear "Check failed" header with tool name. - Shows how to auto-fix (where applicable). - Shows verify command. - Writes to GITHUB_STEP_SUMMARY in CI for job summaries. Update tox environments to use the wrapper script. Ref: ethereum#2041
Remove tox_helpers.py and its entry points, now replaced by scripts/fast_checks.py: - pyspelling_soft_fail: replaced by spellcheck (via codespell). - markdownlintcli2_soft_fail: replaced by markdownlint check. - validate_changelog: replaced by changelog check. Ref: ethereum#2041
Add .github/workflows/fast-checks.yaml that runs all static checks in parallel via matrix jobs: - Spellcheck, Lint, Format, Typecheck, Spec Lint, Lock Check. - Action Lint, Changelog. - Markdown Lint (via dedicated GitHub Action). - SHA Pinned Actions check. Update workflows to use fast-checks as a prerequisite: - test.yaml: removed monolithic static job. - test-docs.yaml: removed duplicate changelog/markdownlint jobs. - benchmark.yaml: added fast-checks prerequisite. - hive-consume.yaml: added fast-checks prerequisite.
- Add `fast-checks` to tox command table in code_standards.md. - Document fix hints feature with example output. - Update recommended commands to use `fast-checks`. - Add pre-commit hooks table to precommit.md. - Update PR template to use `fast-checks`. - Update EIP author manual to use `fast-checks`. - Add .pre-commit-config.yaml for local development.
- Rename tox env `fast-checks` → `check` for consistency - Rename workflow `fast-checks.yaml` → `check.yaml` - Update all workflow refs (`uses:`, `needs:`) to use `check` - Update GitHub Actions to use `tox -e check` - Add fix hints showing `just fix` / `make fix` for lint/format - Create Justfile with recipes delegating to tox - Keep `static` as backwards-compat alias for `check`
Add Makefile as alternative entry point for running checks. All targets delegate to tox (source of truth). Includes `make help` for discoverability.
- Update all `fast-checks` → `check` references in docs - Update CI workflow link to check.yaml - Show just/make/tox options in code standards table - Create docs/dev/build_tools.md with command reference - Update PR template and EIP authors manual
Run all static checks in parallel via just/make for ~2x speedup (~6s → ~3s) and better error reporting (all checks run even if one fails).
c7ebf91 to
c02c126
Compare
- Enable auto-fix hints to appear in CI job summaries by passing the GITHUB_STEP_SUMMARY env var through to tox environments. - fix(ci): don't double-wrap fix hints in code blocks. The fix_hint field already contains markdown code blocks, so write_github_summary shouldn't wrap it in another code block. - fix(ci): simplify verify hint to match PR template style. Show single line: just check # or: make check, uvx tox -e <check>. - fix(ci): make render workflow depend on check workflow.
This reverts commit b669a88.
c02c126 to
c309c56
Compare
|
Someone asked an excellent question via DM:
Essentially we could consider removing tox and rely on just or make as a check runner. However, as-is, I would keep tox as it still provides some useful functionality in both the old and new setups.
I'd keep tox coz it still has some nice orchestration features:
Perhaps I'm missing something else? |
| sha-pinned-actions: | ||
| name: SHA Pinned Actions | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 | ||
| - name: Ensure SHA pinned actions | ||
| uses: zgosalvez/github-actions-ensure-sha-pinned-actions@6124774845927d14c601359ab8138699fa5b70c3 |
There was a problem hiding this comment.
This is available in the repository settings now, unless this serves a different purpose?
| check: | ||
| uses: ./.github/workflows/check.yaml | ||
|
|
There was a problem hiding this comment.
Hm, instead of duplicating check, would on: workflow_run work?
There was a problem hiding this comment.
Hm, workflow_run has a few security considerations. Perhaps workflow_call might be safer.
There was a problem hiding this comment.
We should pick exactly one build system. I understand the desire to make our project as accessible as possible to external developers, but adding a Makefile/Justfile/meson.build/CMakeLists.txt/package.json/pom.xml/ExecutionSpecs.sln/Cargo.toml file just to support muscle memory for another language is silly.
Every build we add is more maintenance for us. If I want to change the build system, I don't want to have to test N different entry points.
Ideally we should have a nice short "Getting Started" section of our readme that includes installation instructions, common commands, and links to further documentation for advanced usage. If a developer can't be bothered to look in our readme for how to run our tooling, I don't really want their contributions.
There was a problem hiding this comment.
Good point, got a bit carried away here! 😄
There was a problem hiding this comment.
I've done a bit more reading into this tool. It's not as bad as I feared. You can't sneak a hook into post-checkout silently; the user needs to rerun pre-commit install.
It still makes me uncomfortable, and I would really rather not promote it, but I'll no longer die on this hill.
There was a problem hiding this comment.
Thanks for checking it out, this was unintentionally committed, sorry about that!
There was a problem hiding this comment.
If we're doing something like this anyway, why not make a build.py in the root directory and just use that instead of just/make/tox?
There was a problem hiding this comment.
I think this approach is now overkill.
| ## Formatting and Line Length | ||
|
|
||
| The Python code in @ethereum/execution-spec-tests is formatted with `ruff` with a line length of 100 characters. | ||
| The Python code in @ethereum/execution-specs is formatted with `ruff` with a line length of 100 characters. |
There was a problem hiding this comment.
| The Python code in @ethereum/execution-specs is formatted with `ruff` with a line length of 100 characters. | |
| The Python code in @ethereum/execution-specs is formatted with `ruff` with a line length of 79 characters. |
SamWilsn
left a comment
There was a problem hiding this comment.
Very old review I forgot to submit ^^;;
| check: | ||
| uses: ./.github/workflows/check.yaml | ||
|
|
There was a problem hiding this comment.
Hm, workflow_run has a few security considerations. Perhaps workflow_call might be safer.
|
Will this PR get updated to remove tox? The PR was opened 2026-01-29, which is the day where we decided in the team meeting to get rid of tox (see gemini summary of 2026-01-29, although gemini phrased it differently), but it still includes tox. Issue #2041 is a bit older so that probably can be closed/reworked when tox is removed |
Yes, I'd like to update this PR to remove In the same testing call, this was deemed low priority and added undue stress on the team, so it was put on hold. I can update if we'd like to prioritize it. Feel free to bring it up in the testing meeting. We did agree to remove |
|
I personally would prefer |
|
Thanks a lot @SamWilsn for your review and @felix314159 for your comments!
Appreciate the feedback, I personally feel that just's UX improvements make it worth it (simple argument pass-through, tasks split by group in help output, no .PHONY, no tabs) and we don't need the additional complexity of a build system. I definitely got a bit carried away in this PR. And made sweeping changes across the build system and CI. I made a more focussed proposal (just a suggestion!) for replacing tox with just in: |
🗒️ Description
This PR improves static check developer experience and introduces finer-grained tox environments for static checks as were configured in ethereum/execution-spec-tests pre-Weld.
It offers 4 main improvements to dev-ex:
Flexible entry point - User can choose their preferred "check" (aka recipe) runner (esp. helpful for non-Python devs):
With this PR, I recommend replacing
uvx tox -e staticwith:These run:
uvx tox --parallel -e lint,format,typecheck,markdownlint,spec-lint,spellcheck,actionlint,changelog,lockcheck, NOTuvx tox -e checkwhich is not parallelized.Parallelization - Each one of the commands in the
toxstaticenv is now a separatetoxenv. This means they can be ran in parallel:i. Faster (~6s → ~3s warm, with mypy cache and
.tox/present).ii. All checks run; they don't stop upon first fail (particularly annoying in CI).
Tips upon fail - Checks are wrapped in a helper
scripts/fast_checks.pythat:i. Provides auto-fix hints on failure (shown on command-line and in CI job summaries, screenshot below).
ii. Uses a soft-fail mode for markdown-cli2 (locally, this was previously in
packages/testing/src/execution_testing/cli/tox_helpers.py, now delted).Fix helper recipe - Adds a "fix"
just/make"recipe" for saferuff(ruff check --fixandruff format):This PR also:
staticchecks moved out oftest.yamltocheck.yamland is configured to ensures no workflows get triggered ifcheck.yamlfails. This involves some unavoidable duplication as each workflow must callcheck.yamlindependently to preserve path filtering. This needs discussion.tox -e static; which also hascheckas alias.Fix Hint Github Summary Screenshot
Fix Hint Console Output (also available locally)
From: https://github.com/danceratopz/execution-specs/actions/runs/21474528622
🔗 Related Issues or PRs
Closes #2041
staticinto separate tox environments & dedicatedfast-checksworkflow #2041✅ Checklist
toxchecks to avoid unnecessary CI fails, see also Code Standards and Enabling Pre-commit Checks:uvx tox -e statictype(scope):.mkdocs servelocally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.Cute Animal Picture