Improve contributor expereince: Justfile/Taskfile for local tasks + custom pre-commit hooks

[mishamsk]

Hi all,

This is based on my first contribution experience recently.

I was wondering if you'd accept a PR with a justfile or taskfile that will make local development easier. Specifically:

• Have shorthands for individual commands like cargo clippy --workspace --all-targets --all-features -- -D warnings or even entire set of tasks to run before pushing a commit
• Allow running local ecosystem checks with one command. Despite the docs, it is not "just" one command. To run locally one needs:
  • Stash changes (or block execution if corktree is not clean)
  • Checkout main, pull
  • Build with --test or test-rules feature enabled (otherwise the execution fails due to RUF9 exclusion on cli)
  • Move the binary to some place
  • Checkout feature branch, build it
  • Run the command from the documentation supplying the two built binaries
• Command to delete snap.new files with confirmation
• There could be more helpful tasks for new contributors, e.g. setting up the environment. Theoretically cargo dev can be wrapped in such tasks and provide nice self-documenting experience
• Also, slightly unrelated, but I suggest creating a custom pre-commit hook that will check for forgotten snapshot.new files. I've built such hooks in my other projects, works like a charm

To be clear, as I am planning to continue contributing I will create such file for myself anyway. But if you think it is worth adding to the repo - I wanted to check if there is any preference for the actual tool. I usually use go-task, but I have a hunch you'd want a rust based solution, so just.

P.S. here is a an example of such file in my lexer project

[MichaReiser]

Hi @mishamsk

Thanks for opening this discussion. We do use just in some projects (e.g. the vs code extension) and I'm not opposed to adding specific tasks if others find them useful too.

Some notes on the tasks you proposed

Command to delete snap.new files with confirmation

This can be done with cargo insta reject

There could be more helpful tasks for new contributors, e.g. setting up the environment. Theoretically cargo dev can be wrapped in such tasks and provide nice self-documenting experience

What would you run as part of this? It should be sufficient to install rustup/rust

Also, slightly unrelated, but I suggest creating a custom pre-commit hook that will check for forgotten snapshot.new files. I've built such hooks in my other projects, works like a charm

I'm more hesitant about this because I don't want to own another pre-commit hook but I'm in general against pre-commit hehe... I might just be the wrong audience here.

cargo clippy --workspace --all-targets --all-features -- -D warnings

I only ever use cargo clippy --fix --allow-staged

Allow running local ecosystem checks with one command. Despite the docs, it is not "just" one command. To run locally one needs:

I'm not sure if this is a good fit for a justfile because you normally only want to build main exactly once. At least that's what I do:

• Build main and move the binary somewhere or install the latest ruff version with uv and use that binary as reference
• build the local branch and run the ecosystem check

The reason I do it this way is because switching branches invalidates the rustc cache, resulting in slower iteration speed overall.

Either way. I'm open to it but I only want to introduce it if there are enough commands that are useful in general vs commands that are very specific to a specific workflow.

[mishamsk]

tl;dr;

I think I'll start building local justfile for myself and share it when I stabilize at least a couple of commands. Would be easier to judge if it worth including for everyone.

Now answering your questions:

This can be done with cargo insta reject

Yes, that's true. I thought it may be helpful for those unfamiliar with insta/snapshot testing - instead of remembering insta API, just can provide a documented command. But that's speculative of course

What would you run as part of this? It should be sufficient to install rustup/rust

I would assume that rustup & uv are installed on the system as a prerequisite, but other tools are not. So basically creating a command that matches CONTRIBUTING.md - prerequisites:

• Checking for insta, nextest and installing it if missing
• installing pre-commit via uv
• running pre-commit install (unless you'll nuke it altogether, see my comment rgd pre-commit below)

I'm more hesitant about this because I don't want to own another pre-commit hook but I'm in general against pre-commit hehe... I might just be the wrong audience here.

I see. I found it useful in my commercial project that heavily relies on snapshots (also a parser-static analyzer-formatter etc.). I remembered about it when I saw your commit that removed snap files from my PR and thought I accidentally left them.

Unlike tasks, I can't "inject" my own pre-commit hooks easily, so obviously this one would only be possible if you approve its inclusion for everyone.

I only ever use cargo clippy --fix --allow-staged

I've followed the contributors guide here. But the goal is to allow contributors to run the "same exact" checks as CI does locally before committing. Obviously not everything can be run locally, and only the host target is tested, but IMHO this improves velocity instead of waiting for CI checks to pass.

So whatever the actual command will be, I would try to follow GitHub workflow as close as possible.

install the latest ruff version with uv and use that binary as reference

I believe this is not possible. Release binaries do not have RUF9 test rules, and ruff_ecosystem runner executes a command that has --ignore RUF9 which fails. So one has to have a test build to compare too.

The reason I do it this way is because switching branches invalidates the rustc cache, resulting in slower iteration speed overall.

Yes, compile time will be an issue here, I agree. I'll think of a user friendly alternative. Theoretically it may be possible to mimic CI workflow and access Github main builds and download them... I'll investigate at some point

[mishamsk]

@MichaReiser so here is what I got so far:

# Import the user's Justfile if it exists
import? '~/.justfile'
import? '~/justfile'

# Show the list of available commands
default:
  just --list

# Runs clippy on the workspace (checks only)
lint:
  cargo clippy --workspace --all-targets --all-features --locked -- -D warnings
  cargo clippy -p ruff_wasm -p red_knot_wasm --target wasm32-unknown-unknown --all-features --locked -- -D warnings

# Lint & apply fixes. Modifies the working tree!
lint-fix:
  cargo clippy --fix --allow-staged

# Lints using clippy and runs pre-commit hooks
check: lint
  pre-commit run --all-files --show-diff-on-failure

# Run non-doc tests using insta & nextest (mimics linux CI)
test:
  cargo insta test --all-features --unreferenced reject --test-runner nextest

# Run all tests (mimics linux CI)
test-all: test
  RUSTDOCFLAGS="-D warnings" cargo doc --all --no-deps
  RUSTDOCFLAGS="-D warnings" cargo doc --no-deps -p red_knot_python_semantic -p red_knot -p red_knot_test -p ruff_db --document-private-items

# Run ecosystem checks. Pass the base ruff executable as an argument. It must be compiled with `-F test-rules`!
test-eco-check base_executable="ruff" repo_cache='':
  @echo "Running ecosystem checks. Using '{{base_executable}}' as the base executable..."
  uvx --from ./python/ruff-ecosystem ruff-ecosystem {{ if repo_cache != '' { "--cache " + repo_cache } else { '' } }} check {{base_executable}} "./target/debug/ruff"

# Run ecosystem checks (preview). Pass the base ruff executable as an argument. It must be compiled with `-F test-rules`!
test-eco-check-preview base_executable="ruff" repo_cache='':
  @echo "Running ecosystem checks. Using '{{base_executable}}' as the base executable..."
  uvx --from ./python/ruff-ecosystem ruff-ecosystem {{ if repo_cache != '' { "--cache " + repo_cache } else { '' } }} --force-preview check {{base_executable}} "./target/debug/ruff"

[MichaReiser]

Sorry, I'm currently a bit swampped with notifications/issues/PRs that I need to look at. I don't know when I've time to look into this.