Closed rrybarczyk closed 1 week ago
Here are the run times for the scripts:
clippy-on-all-workspaces.sh
containing cargo clippy
, cargo test
, and cargo +nightly fmt
:./scripts/clippy-on-all-workspaces.sh 58.09s user 1.76s system 145% cpu 41.107 total
clippy-on-all-workspaces.sh
containing cargo clippy
and cargo +nightly fmt
(cargo test
removed): ./scripts/clippy-on-all-workspaces.sh 2.90s user 0.52s system 74% cpu 4.582 total./scripts/sv2-header-check.sh 0.92s user 0.24s system 88% cpu 1.307 total
Total run time of the ./.githooks/pre-push
:
cargo test
in clippy-on-all-workspaces.sh
: ./.githooks/pre-push 58.62s user 1.97s system 144% cpu 42.058 total
cargo test
in clippy-on-all-workspaces.sh
: ./.githooks/pre-push 3.91s user 0.82s system 89% cpu 5.292 total
@plebhash, do you want to keep the cargo test
or remove? I have no preference, just comes down to a 1 minute run time with tests vs a 4 second run time without and what we want the user experience to be.
I am experiencing the following issues in running ./scripts/sv2-header-check.sh
:
zsh: ./scripts/sv2-header-check.sh: bad interpreter: /usr/bin/sh: no such file or directory
#! /usr/bin/sh
at the top of the file to #!/bin/sh
. However, will changing this break on other user's systems? The #!/bin/sh
is what clippy-on-all-workspaces.sh
uses.scripts/sv2.h
file generated by this script does not get deleted after the run has completed.My solution is to add rm -f scripts/sv2.h
at the top of the file before running, which deletes the file if it exists.
@plebhash, thoughts on splitting up the clippy-on-all-workspaces
into the following stand alone bash scripts:
scripts/clippy
: Runs cargo clippy
on all workspaces.scripts/test
: Runs cargo test
on all workspaces.scripts/fmt
: Runs cargo nightly fmt
on all workspaces.Or is there a reason why these are all grouped together? If we choose to keep in same file, we should rename to clippy-fmt-and-test.sh
or something.
@plebhash, thoughts on splitting up the
clippy-on-all-workspaces
into the following stand alone bash scripts:* `scripts/clippy`: Runs `cargo clippy` on all workspaces. * `scripts/test`: Runs `cargo test` on all workspaces. * `scripts/fmt`: Runs `cargo nightly fmt` on all workspaces.
Or is there a reason why these are all grouped together? If we choose to keep in same file, we should rename to
clippy-fmt-and-test.sh
or something.
I think they are grouped just for convenience. I would keep them together, renaming the script in clippy-fmt-and-test.sh
as you suggested.
Background
act
is a tool that works with Docker to run GitHub Actions workflows locally, designed to be run before pushing changes. The.githooks/pre-push
usesact
to run some GitHub workflows with% ./.githooks/pre-push origin https://github.com/stratum-mining/stratum.git
.The currently defined workflows in this
pre-push
script are:message_generator_check
(this is not an existing workflow, probably trying to referencemessage-generator-test
)sv2_header_check
fmt
clippy-check
ci
During the 2024.07.02 dev meet, the team decided that the
pre-push
should only containing more commonly needed workflows, like for formatting. Additionally, because of long run times when usingact
, the usage ofact
should be replaced with calling the bash scripts in thescripts/
directory in the repo root. This will keep thepre-push
runtime shorter, and then any more specific workflows needed can be run manually by executing the bash scripts in thescripts/
directory on thedev
branch.Problem
pre-push
script should only contain workflows that are more commonly run, like for linting.message_generator_check
is causing thepre-push
to fail.act
are long.Solution
In
.githooks/pre-push
, replace the use ofact
with calls to the following bash scripts in thescripts/
directory on thedev
branch:clippy-on-all-workspaces.sh
: Runscargo clippy
,cargo test
, andcargo +nightly fmt
.sv2-header-check.sh
: Ensures that thesv2.h
file generated bybuild_header.sh
is in sync with the commitedprotocols/v2/sv2-ffi/sv2.h
.