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

ci: improve github workflows #190

Merged
merged 2 commits into from
Jun 21, 2024
Merged

Conversation

koushiro
Copy link
Contributor

  • split test workflow into lint and test parts
  • add cache and sccache
  • use stable toolchain for rustfmt and clippy

- split test workflow into lint and test parts
- add cache and sccache
- use stable toolchain for rustfmt and clippy
@koushiro
Copy link
Contributor Author

koushiro commented Jun 20, 2024

I actually prefer to use consistent commands to build repo in local dev and CI, such as using bash and makefile directly, like:

Before

CI

      - name: Install toolchain
        uses: dtolnay/rust-toolchain@stable
        with:
          toolchain: 1.78.0
          components: rustfmt, clippy

      - name: Check format
        run: cargo fmt --all -- --check

      - name: Check clippy
        run: cargo clippy --all-targets --all-features -- -D warnings

After

CI

      - name: Install toolchain
        run: make setup

      - name: Check format
        run: make fmt-check

      - name: Check clippy
        run: make clippy-release

Makefile

.PHONY: setup
# Setup development environment
setup:
    bash ./scripts/setup-dev.sh

.PHONY: fmt-check fmt
# Check the code format
fmt-check:
    taplo fmt --check
    cargo fmt --all -- --check
# Format the code
fmt:
    taplo fmt
    cargo fmt --all

.PHONY: clippy clippy-release
# Run rust clippy with debug profile
clippy:
    cargo clippy --workspace --all-targets --all-features -- -D warnings
# Run rust clippy with release profile
clippy-release:
    cargo clippy --release --workspace --all-targets --all-features -- -D warnings

setup-dev.sh

#!/bin/bash

function install_rustup {
  echo "Installing Rust toolchain..."
  if rustup --version &> /dev/null; then
    echo "Rust toolchain has been installed"
  else
    curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y --default-toolchain none
    source $HOME/.cargo/env
  fi
  rustup show
}

install_rustup

We already have rust-toolchain.toml in the repo, so we don't need to specify the toolchain version and components again in the CI.

@xlc what do you think?

@xlc
Copy link
Member

xlc commented Jun 21, 2024

dtolnay/rust-toolchain doesn't support the toolchain file dtolnay/rust-toolchain#77

the suggestions looks good

@koushiro
Copy link
Contributor Author

@xlc If you agree with the above suggestion, maybe we could merge this PR first and I'll do the rest in a later PR.
Or I can apply the above command directly in this PR?

@xlc xlc merged commit d466f18 into AcalaNetwork:master Jun 21, 2024
1 check passed
@koushiro koushiro deleted the improve-ci branch June 21, 2024 07:15
koushiro referenced this pull request in alt-research/subway Jun 21, 2024
* ci: improve github workflows

- split test workflow into lint and test parts
- add cache and sccache
- use stable toolchain for rustfmt and clippy

* remove useless env
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants