Feature/precommit hooks#517
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces pre-commit hooks using prek, updates various tool versions in .mise.toml, migrates repository references from the earthly organization to EarthBuild/earthbuild, and upgrades several Go dependencies in go.mod. The reviewer identified three critical issues: invalid module paths and non-existent versions for both golangci-lint and gofumpt in .mise.toml which will cause installation failures, and a mismatched variable name (BUILDKIT_BRANCH instead of BUILDKIT_GIT_BRANCH) in buildkitd/Earthfile that results in empty branch names in logs.
| echo "looking up branch $BUILDKIT_GIT_BRANCH"; \ | ||
| buildkit_sha1=$(git ls-remote --refs -q https://github.com/$BUILDKIT_GIT_ORG/buildkit.git "$BUILDKIT_GIT_BRANCH" | awk 'BEGIN { FS = "[ \t]+" } {print $1}'); \ | ||
| echo "pinning github.com/earthly/buildkit@${BUILDKIT_BRANCH} to reference git sha1: $buildkit_sha1"; \ | ||
| echo "pinning github.com/${BUILDKIT_GIT_ORG}/buildkit@${BUILDKIT_BRANCH} to reference git sha1: $buildkit_sha1"; \ |
There was a problem hiding this comment.
The variable ${BUILDKIT_BRANCH} is used here, but the argument defined at the top of the target is BUILDKIT_GIT_BRANCH. This will result in an empty string being printed for the branch name. Please use ${BUILDKIT_GIT_BRANCH} instead.
echo "pinning github.com/${BUILDKIT_GIT_ORG}/buildkit@${BUILDKIT_GIT_BRANCH} to reference git sha1: $buildkit_sha1"; \
|
@copilot resolve the merge conflicts in this pull request |
| id = "golangci-lint-fix" | ||
| name = "golangci-lint (auto-fix)" | ||
| language = "system" | ||
| entry = "golangci-lint run --fix" |
There was a problem hiding this comment.
We should be running the earth targets for this otherwise you're picking up whatever version of lint is installed on your box.
There was a problem hiding this comment.
@gilescope I agree but I thought git hooks need to be as fast as possible. Another solution would be to move it to a shell script which is used in hooks and also Earthfile +lint target.
|
Are these several things going on here? Is this the minimal changes for the precommit? Could we get these split out into smaller PRs please? |
BUILDKIT_BRANCH was referenced but only BUILDKIT_GIT_BRANCH was defined, causing an empty string to be printed. Also uses BUILDKIT_GIT_ORG instead of the hardcoded 'earthly' org.
danielschlegel
force-pushed
the
feature/precommit-hooks
branch
from
a9634ba to
9ef9821
Compare
➖ Are we earthbuild yet?No change in "earthly" occurrences 📈 Overall Progress
Keep up the great work migrating from Earthly to Earthbuild! 🚀 💡 Tips for finding more occurrencesRun locally to see detailed breakdown: ./.github/scripts/count-earthly.shNote that the goal is not to reach 0. |
danielschlegel
force-pushed
the
feature/precommit-hooks
branch
from
9ef9821 to
b26d605
Compare
- Add prek.toml with hooks for gofumpt, golangci-lint (auto), golangci-lint --fix (manual only), and shellcheck. - Add scripts/golangci-lint.sh as the single source of truth for the pinned golangci-lint version; used by both prek hooks and the Earthfile. - Refactor +lint into +lint-deps (shared setup) and +lint targets. - Add +lint-fix target that runs golangci-lint --fix and saves corrected files back to the working tree via SAVE ARTIFACT ... AS LOCAL.
danielschlegel
force-pushed
the
feature/precommit-hooks
branch
from
b26d605 to
9cb24c5
Compare
2 participants
Add prek pre-commit hooks for lint auto-fix
What
Introduces prek as a Git pre-commit hook manager to automatically fix Go linting and formatting issues before every commit.
Changes
prek.toml(new) — declares two hooks that run on staged.gofiles:.golangci.yaml.--fix— applies all auto-fixable lint corrections. Runs against full packages (pass_filenames = false) so type-checking is correct..mise.toml— adds three new tools somise installsets up the full dev environment in one step:prek 0.4.3— the hook manager itself.golangci-lint v2.11.4(viago:backend) — pinned to match the version inEarthfile.gofumpt v0.10.0(viago:backend) — pinned to the current latest release.Both tool versions carry
# renovate:comments so Renovate will keep them in sync automatically.CONTRIBUTING.md— adds a Pre-commit hooks section documenting the two-command setup:How it works
On
git commit, prek runs the hooks against staged.gofiles only (non-Go commits are unaffected). If a hook modifies a file, the commit is blocked so you can review the diff, re-stage, and commit again. To pre-apply all fixes before staging:prek run --all-files.