build: integrate pre-commit hooks with upstream files filtering support #1121

powerslider · 2025-08-14T15:53:38Z

Why this should be merged

Check #1120 for the motivations.

How this works

Add .pre-commit-config.yaml.
Configured hooks:
- Baseline golangci-lint (v1) at pre-commit using .golangci.yml at pre-commit stage.
- avalanche-golangci-lint via scripts/lint.sh at pre-commit stage.
- Standard pre-commit-hooks:
  - trailing-whitespace
  - end-of-file-fixer
  - check-merge-conflict
  - check-yaml
  - check-toml
  - check-json
- mdformat with mdformat-gfm and mdformat-frontmatter
Exclude intentionally invalid fixtures: rpc/testdata/invalid-*.json.
Add support for filtering filenames based on scripts/upstream_files.txt (non-negated entries).
Add support for skipping directories, non-text files (file/grep heuristic), and .bin files.
Update scripts/shellcheck.sh and scripts/actionlint.sh to avoid unbound vars under set -u.

How this was tested

Manually by installing the pre-commit hooks and running them at pre-commit stage.

Need to be documented?

Only dev documentation.

Need to update RELEASES.md?

no

resolves #1120

Signed-off-by: Tsvetan Dimitrov ([email protected])

alarso16

I would personally use a pre-commit hook, but it might make sense to review this #1117 is merged

scripts/lint.sh

precompile/contracts/warp/README.md

scripts/upstream_files.txt

…tering - Add .pre-commit-config.yaml. - Configured hooks: - Baseline golangci-lint (v1) at pre-commit using .golangci.yml at pre-commit stage. - [COMMENTED OUT] Extra golangci-lint via scripts/lint.sh at pre-commit stage. - Full repo suite at pre-push stage. - Standard pre-commit-hooks: - trailing-whitespace - end-of-file-fixer - check-merge-conflict - check-yaml - check-toml - check-json - mdformat with mdformat-gfm and mdformat-frontmatter - Exclude intentionally invalid fixtures: rpc/testdata/invalid-*.json. - Add support for filtering filenames based on scripts/upstream_files.txt (non-negated entries). - Add support for skipping directories, non-text files (file/grep heuristic), and .bin files. - Update scripts/shellcheck.sh and scripts/actionlint.sh to avoid unbound vars under set -u. resolves #1120 Signed-off-by: Tsvetan Dimitrov ([email protected])

… as a pre-push hook

…vaLabs licence header

…f-file-fixer check

cmd/abigen/namefilter.go

log/format.go

rpc/testdata/invalid-syntax.json

.pre-commit-config.yaml

alarso16 · 2025-08-19T16:42:21Z

.pre-commit-config.yaml

+        exclude: ^rpc/testdata/invalid-.*\.json$
+
+  # Markdown formatter
+  - repo: https://github.com/hukkin/mdformat


What's standard here? I'm not sure if we want to make these changes just for the pre-commit hooks (added arbitrarily) if we're not also going to lint for them

This is the style guide it follows https://mdformat.readthedocs.io/en/stable/users/style.html

I agree with @alarso16 re aligning pre-commit and linter checks. IMO the pre-commit is the fast-fail while the linter is the gatekeeper.

license_header.yml

accounts/abi/bind/bind_test.go

…ring for the hooks

alarso16

Just some questions to clarify

scripts/lint.sh

RELEASES.md

predicate/Results.md

sync/README.md

ARR4N · 2025-09-04T11:12:15Z

.github/CONTRIBUTING.md

(fyi) I'm not reviewing this file, on the assumption that it's only formatting. Please let me know if that's an incorrect assumption.

ARR4N · 2025-09-04T11:15:22Z

cmd/abigen/namefilter.go

 //
 // Much love to the original authors for their work.
 // **********
-


I'm 99% sure that this is not what we want. By removing the blank space it will force the copyright header to become part of the package comment. Run godoc locally to check.

I suspect that the header hasn't been configured correctly.

ARR4N · 2025-09-04T11:17:21Z

docs/releasing/README.md

As above, not reviewing as assuming that it's only formatting.

ARR4N · 2025-09-04T11:21:18Z

precompile/contracts/warp/README.md

-4. An off-chain relayer queries the validators for their signatures of the message and aggregates the signatures to create a `SignedMessage`
-5. The off-chain relayer encodes the `SignedMessage` as the [predicate](#predicate-encoding) in the AccessList of a transaction to deliver on blockchain B
-6. The transaction is delivered on blockchain B, the signature is verified prior to executing the block, and the message is accessible via the Warp Precompile's `getVerifiedWarpMessage` during the execution of that transaction
+1. Warp Precompile emits an event / log containing the `UnsignedMessage` specified by the caller of `sendWarpMessage`


Is there no linter that can instead check that the numbers are up to date? This forces someone to refer to the rendered version, which defeats the point of markdown being readable.

ARR4N · 2025-09-04T11:22:00Z

predicate/Predicate.md

Why are there 2 new markdown files in this directory?

ARR4N · 2025-09-04T11:23:11Z

scripts/actionlint.sh

 set -euo pipefail

-go run github.com/rhysd/actionlint/cmd/[email protected] "${@}"
+if [[ $# -gt 0 ]]; then


Is this a shellcheck recommendation? My bash-fu isn't strong enough to review this comfortably.

ARR4N · 2025-09-04T11:24:39Z

scripts/shellcheck.sh

 done

-find "${REPO_ROOT}" \( "${IGNORED_CONDITIONS[@]}" \) -o -type f -name "*.sh" -print0 | xargs -0 "${SHELLCHECK}" "${@}"
+if [[ $# -gt 0 ]]; then


As with run_task.sh, why this pattern? Did shellcheck recommend it?

ARR4N · 2025-09-04T11:27:31Z

.pre-commit-config.yaml

+        exclude: ^rpc/testdata/invalid-.*\.json$
+
+  # Markdown formatter
+  - repo: https://github.com/hukkin/mdformat


I agree with @alarso16 re aligning pre-commit and linter checks. IMO the pre-commit is the fast-fail while the linter is the gatekeeper.

ARR4N · 2025-09-04T11:34:07Z

scripts/filter_precommit_files.sh

I don't think we should be introducing even more bash scripts when a Go binary can do the same job, but in a much more readable fashion. Why not have the pre-commit entrypoint be something like go run ./scripts/foo --checker="bar"?

If we want to avoid a new directory for every script then we could just create a cobra app and add scripts/main.go as the entry and a new Go file with separate sub-command for what otherwise would have been a new shell script. Then the entrypoint would be go run ./scripts precommit filter --checker="bar".

powerslider self-assigned this Aug 14, 2025

powerslider requested review from a team and maru-ava as code owners August 14, 2025 15:53

alarso16 reviewed Aug 14, 2025

View reviewed changes

scripts/lint.sh Outdated Show resolved Hide resolved

precompile/contracts/warp/README.md Show resolved Hide resolved

scripts/upstream_files.txt Show resolved Hide resolved

powerslider force-pushed the powerslider/1120-pre-commit-hooks branch from cb17607 to 4a571be Compare August 15, 2025 13:07

powerslider added 3 commits August 15, 2025 16:13

fix: revert scripts/lint.sh due to not being practical to run locally…

c310875

… as a pre-push hook

Merge branch 'master' into powerslider/1120-pre-commit-hooks

fba660f

fix: add normalization steps when reading upstream_files.txt

250c431

powerslider force-pushed the powerslider/1120-pre-commit-hooks branch from a754142 to 250c431 Compare August 15, 2025 18:09

powerslider added 3 commits August 18, 2025 12:23

Merge branch 'master' into powerslider/1120-pre-commit-hooks

3a41223

fix: add a blank line to please the license_header linter after the A…

d739d3b

…vaLabs licence header

fix: revert unimpactful changes in lint.sh script

c0c59e3

powerslider force-pushed the powerslider/1120-pre-commit-hooks branch from 3e0559c to c82a81b Compare August 18, 2025 11:41

powerslider added 2 commits August 18, 2025 13:45

fix: more license fixes

6cfa338

fix: revert adding a new line after //********** because of the end-o…

fab273b

…f-file-fixer check

powerslider force-pushed the powerslider/1120-pre-commit-hooks branch 3 times, most recently from d362beb to f81db78 Compare August 18, 2025 12:09

fix: more license fixes

ea3388d

powerslider force-pushed the powerslider/1120-pre-commit-hooks branch from f81db78 to ea3388d Compare August 18, 2025 12:10

powerslider added 2 commits August 19, 2025 14:42

Merge branch 'master' into powerslider/1120-pre-commit-hooks

9a632b4

Merge branch 'master' into powerslider/1120-pre-commit-hooks

45b9885

alarso16 reviewed Aug 19, 2025

View reviewed changes

fix: enable avalanche_golangci_lint hook

bcda734

powerslider force-pushed the powerslider/1120-pre-commit-hooks branch from 56d2b9e to bcda734 Compare August 19, 2025 17:23

alarso16 requested changes Aug 19, 2025

View reviewed changes

accounts/abi/bind/bind_test.go Show resolved Hide resolved

powerslider force-pushed the powerslider/1120-pre-commit-hooks branch 3 times, most recently from aeaa6d9 to b13b98b Compare August 19, 2025 19:08

powerslider force-pushed the powerslider/1120-pre-commit-hooks branch from b13b98b to 26c4001 Compare August 19, 2025 19:10

fix: treats both relative and absolute repo paths the same when filte…

4819f5c

…ring for the hooks

powerslider force-pushed the powerslider/1120-pre-commit-hooks branch from 26c4001 to 4819f5c Compare August 19, 2025 19:14

powerslider added 2 commits August 19, 2025 22:26

docs: add README docs

f53e9d1

fix: add support to the filter script for absolute and ./ paths

0565060

alarso16 reviewed Aug 19, 2025

View reviewed changes

scripts/lint.sh Outdated Show resolved Hide resolved

scripts/lint.sh Outdated Show resolved Hide resolved

powerslider added 2 commits August 25, 2025 14:56

Merge branch 'master' into powerslider/1120-pre-commit-hooks

6394b7e

fix: revert changes on lint.sh

409def6

JonathanOppenheimer approved these changes Aug 27, 2025

View reviewed changes

RELEASES.md Show resolved Hide resolved

predicate/Results.md Show resolved Hide resolved

sync/README.md Show resolved Hide resolved

powerslider added 3 commits August 27, 2025 23:45

Merge branch 'master' into powerslider/1120-pre-commit-hooks

9d9c631

Merge branch 'master' into powerslider/1120-pre-commit-hooks

9afc8c4

Merge branch 'master' into powerslider/1120-pre-commit-hooks

8879363

ARR4N requested changes Sep 4, 2025

View reviewed changes

Merge branch 'master' into powerslider/1120-pre-commit-hooks

b025d27

build: integrate pre-commit hooks with upstream files filtering support #1121

Are you sure you want to change the base?

build: integrate pre-commit hooks with upstream files filtering support #1121

Uh oh!

Conversation

powerslider commented Aug 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why this should be merged

How this works

How this was tested

Need to be documented?

Need to update RELEASES.md?

Uh oh!

alarso16 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

alarso16 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

powerslider commented Aug 14, 2025 •

edited

Loading