Skip to content

chore: begin replacement of Make and ./scripts/gear.sh with Just#4862

Closed
liferooter wants to merge 17 commits intomasterfrom
gs-rework-scripts
Closed

chore: begin replacement of Make and ./scripts/gear.sh with Just#4862
liferooter wants to merge 17 commits intomasterfrom
gs-rework-scripts

Conversation

@liferooter
Copy link
Copy Markdown
Contributor

@liferooter liferooter commented Sep 23, 2025

  • Introduces justfile as an alternative to existing Makefile and ./scripts/gear.sh.
  • Implements some commonly used Make targets as Just recipes.
  • Replaces usage of ./scripts/gear.sh with usage of Just in some parts of CI.
  • Inline some usages of ./scrpts/gear.sh in CI where there's no need for abstraction into a script.

@liferooter liferooter requested a review from ark0f September 23, 2025 09:52
@liferooter liferooter added the D5-tooling Helper tools and utilities label Sep 23, 2025
@semanticdiff-com
Copy link
Copy Markdown

semanticdiff-com Bot commented Sep 23, 2025

Review changes with  SemanticDiff

Changed Files
File Status
  .github/workflows/comparison-table.yml  36% smaller
  .github/workflows/time-consuming-tests.yml  29% smaller
  .github/workflows/check.yml  14% smaller
  .github/actions/install-cargo-extensions/action.yml  0% smaller
  .github/workflows/build-linux.yml  0% smaller
  .github/workflows/build.yml  0% smaller
  .github/workflows/test-measurements.yaml  0% smaller
  just/clippy.just Unsupported file format
  just/ethexe.just Unsupported file format
  just/lib.just Unsupported file format
  justfile Unsupported file format
  shell.nix Unsupported file format

@liferooter liferooter added the A0-pleasereview PR is ready to be reviewed by the team label Sep 23, 2025

- name: "Check clippy"
run: ./scripts/gear.sh clippy gear --all-targets --all-features --locked
run: just clippy native
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

its more likely
just clippy :: { workspace ; examples } -> not the point that its wasms
but moreover these steps could be merged into one

- name: "Install: Cargo extensions"
uses: ./.github/actions/install-cargo-extensions

- name: "Show: Versioning"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

such simple commands could be nameless in CI

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Other such commands, like "Install: Rust toolchain" or "Install: Foundry" are not nameless.

Comment thread .github/actions/install-cargo-extensions/action.yml
Comment thread justfile
Comment thread just/lib.just
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

My custom ensure-* repices are more powerful than Just's require function.

  • It shows hint that describes how to install the program.
  • It is recipe-specific, not global.
  • It is custom enough to be able to check Cargo extensions.

Comment thread justfile
Comment on lines +29 to +37
# Format code via `rustfmt`
[group('actions')]
fmt:
cargo fmt --all

# Check formatting with `rustfmt`
[group('checks')]
fmt-check:
cargo fmt --all --check
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
# Format code via `rustfmt`
[group('actions')]
fmt:
cargo fmt --all
# Check formatting with `rustfmt`
[group('checks')]
fmt-check:
cargo fmt --all --check
# Format code via `rustfmt`
[group('actions')]
fmt *ARGS:
cargo fmt --all {{ARGS}}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think having a separate fmt-check recipe is better for some reasons:

  • Accepting arguments by fmt recipe is not very useful because the only useful argument that can be passed to it is --check.
  • Recipes with arguments cannot be chained (e.g. we can't run just fmt typos test-doc), so it makes sense to refrain from using recipes with arguments as much as possible.
  • Having a separate fmt-check recipe improves autocompletion for this recipe.
  • Having a separate fmt-check recipe allows to used it as a dependency (like for pre-commit) without causing an argument passing hell like what we have in Makefile and gear.sh now.

Comment thread justfile Outdated
Comment thread justfile
Comment on lines +79 to +84
[group('checks')]
test-doc:
# Running documentation tests
__GEAR_WASM_BUILDER_NO_BUILD=1 \
SKIP_WASM_BUILD=1 \
cargo test --doc --workspace --no-fail-fast
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
[group('checks')]
test-doc:
# Running documentation tests
__GEAR_WASM_BUILDER_NO_BUILD=1 \
SKIP_WASM_BUILD=1 \
cargo test --doc --workspace --no-fail-fast
[group('checks')]
test-doc $__GEAR_WASM_BUILDER_NO_BUILD="1" $SKIP_WASM_BUILD="1":
cargo test --doc --workspace --no-fail-fast

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It was done on purpose. Using long recipe-specific environment variables makes help messages very ugly and also confuses by being the only thing about running commands that is not printed during recipe execution.

Comment thread justfile Outdated
Comment on lines +62 to +76
test: (ensure-cargo "hack") (ensure-cargo "nextest")
# Running workspace tests
cargo nextest run \
--workspace \
--no-fail-fast \
--exclude gclient \
--exclude gcli \
--exclude gsdk \
--exclude gear-authorship \
--exclude pallet-gear-staking-rewards \
--exclude gear-wasm-gen \
--exclude demo-stack-allocations \
--exclude gring \
--exclude runtime-fuzzer \
--exclude runtime-fuzzer-fuzz
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
test: (ensure-cargo "hack") (ensure-cargo "nextest")
# Running workspace tests
cargo nextest run \
--workspace \
--no-fail-fast \
--exclude gclient \
--exclude gcli \
--exclude gsdk \
--exclude gear-authorship \
--exclude pallet-gear-staking-rewards \
--exclude gear-wasm-gen \
--exclude demo-stack-allocations \
--exclude gring \
--exclude runtime-fuzzer \
--exclude runtime-fuzzer-fuzz
test *ARGS: (ensure-cargo "hack") (ensure-cargo "nextest")
# Running workspace tests
cargo nextest run --workspace --all-targets --all-features -E 'not package(/fuzz/)' {{ARGS}}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Simplification is done.

Argument passing is a bad idea I think.

  • It is not very useful, because for running tests for specific crates or with specific options it's better to use cargo nextest directly for better control and more explicitness. The point of the recipe is to give us a short command for a common non-trivial testing task, not to give us an universal script runner, for which always having all these options by default is not a good idea.
  • The same reasons as with fmt-check. Recipes without arguments can be chained, generally cleaner and will never cause argument passing hell.

Comment thread justfile
Comment thread just/clippy.just
# Check all WASM code
wasm:
# Checking all WASM code with Clippy
cargo metadata --no-deps --format-version=1 \
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Consider to use shebang scripts and paste lines directly from ./scripts/src/clippy.sh

@grishasobol
Copy link
Copy Markdown
Member

Consult with @breathx and decided to be closed

@grishasobol grishasobol closed this Apr 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A0-pleasereview PR is ready to be reviewed by the team D5-tooling Helper tools and utilities

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants