[eng quality] pre-commit hook docs

[pavitthrap]

Description

Adds documentation explaining how to locally setup a pre-commit hook

Applicable issues

• fixes [eng quality] implement git hook for rustfmt #3570

[codecov]

Codecov Report

Merging #3586 (dac322d) into develop (d971113) will increase coverage by 2.67%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop    #3586      +/-   ##
===========================================
+ Coverage    28.58%   31.26%   +2.67%     
===========================================
  Files          298      298              
  Lines       275135   275135              
===========================================
+ Hits         78644    86012    +7368     
+ Misses      196491   189123    -7368     

Impacted Files	Coverage Δ
clarity/src/vm/ast/mod.rs	25.15% <0.00%> (-1.91%)	⬇️
stacks-common/src/util/retry.rs	38.51% <0.00%> (-0.75%)	⬇️
src/chainstate/stacks/mod.rs	9.15% <0.00%> (-0.67%)	⬇️
src/chainstate/stacks/auth.rs	10.45% <0.00%> (-0.19%)	⬇️
src/chainstate/stacks/index/cache.rs	13.37% <0.00%> (-0.17%)	⬇️
src/clarity_vm/clarity.rs	37.35% <0.00%> (-0.11%)	⬇️
testnet/stacks-node/src/config.rs	47.95% <0.00%> (-0.07%)	⬇️
src/net/neighbors.rs	31.76% <0.00%> (-0.03%)	⬇️
src/net/download.rs	40.23% <0.00%> (+0.03%)	⬆️
src/net/db.rs	29.32% <0.00%> (+0.05%)	⬆️
... and 98 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[fpbgg]

I followed these instructions then tried to commit an invalid change (adding whitespace to the imports).

% git commit
rustfmt failed: run "cargo fmt --all -- --config group_imports=StdExternalCrate"

I ran the suggested command. I was prompted to provide a commit message as expected.

[donpdonp]

Approved - as is looks good to me but I'm leaving this suggested change:

The script block below is easy to read, keeps the complicated cargo fmt command in one place for future edits, and actually formats the code if the --check fails but does not check it in.

The user is alerted that the code has been formatted and re-running the git commit -a (usually an up arrow from the terminal) will include the format changes. Its also appendable to pre-commit (only exits on failure) for when users might have an existing pre-commit hook.

2. Append the following:

CARGO_FMT="cargo fmt --all -- --config group_imports=StdExternalCrate"

$CARGO_FMT --check
CARGO_FMT_RESULT=$?

if [ $CARGO_FMT_RESULT -eq 1 ]
then
    $CARGO_FMT
    echo 'cargo fmt --all; applied, commit again.'
    exit $CARGO_FMT_RESULT
fi

[pavitthrap]

Approved - as is looks good to me but I'm leaving this suggested change:

The script block below is easy to read, keeps the complicated cargo fmt command in one place for future edits, and actually formats the code if the --check fails but does not check it in.

The user is alerted that the code has been formatted and re-running the git commit -a (usually an up arrow from the terminal) will include the format changes. Its also appendable to pre-commit (only exits on failure) for when users might have an existing pre-commit hook.

2. Append the following:

CARGO_FMT="cargo fmt --all -- --config group_imports=StdExternalCrate"

$CARGO_FMT --check
CARGO_FMT_RESULT=$?

if [ $CARGO_FMT_RESULT -eq 1 ]
then
   $CARGO_FMT
   echo 'cargo fmt --all; applied, commit again.'
   exit $CARGO_FMT_RESULT
fi

Discussed with @donpdonp and agreed there are advantages to both versions (for example, the one I proposed only runs the check command for staged files).

[blockstack-devops]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.