feat: pre commit hooks for linting #41
Conversation
- Add comprehensive pre-commit configuration for Go, Markdown, and shell scripts - Include end-of-file fixing, trailing whitespace removal, and Go formatting - Add lint target to Makefile that runs pre-commit hooks
- Fix end-of-file issues in .pre-commit-config.yaml and www/style.large.css - Remove trailing whitespace from USAGE.md and DOCS.md - Format Go code with gofmt in database.go - Update import aliases with goimports in api.go and errors.go - Clean up go.mod dependencies with go mod tidy
There was a problem hiding this comment.
wait why are we deleting old version hashes
5HT2
left a comment
5HT2
left a comment
There was a problem hiding this comment.
Thank you for the PR! I mostly have a few questions but it otherwise looks good
| github.com/go-redis/redis/v8 v8.11.5 | ||
| github.com/joho/godotenv v1.5.1 | ||
| github.com/nitishm/go-rejson/v4 v4.2.0 | ||
| github.com/redis/go-redis/v9 v9.6.0 |
There was a problem hiding this comment.
Are there breaking changes from v8 to v9 / have you tested the following steps basically
- running an existing build
- upgrading
- rebuilding
- running new redis client version
I can do it for you if not
There was a problem hiding this comment.
tested my deployment and lgtm
There was a problem hiding this comment.
(I did however write my own containerfile using vendor deps instead of make deps)
FROM golang:1.23-alpine AS builder
RUN mkdir -p /heartbeat/config
ADD . /heartbeat
WORKDIR /heartbeat
# Install make, git and other build dependencies
RUN apk add --no-cache make git
ENV PATH "${PATH}:${GOPATH}/bin"
RUN go mod tidy
RUN go mod download
RUN go build -ldflags "-X main.gitCommitHash=6034c5f" -o heartbeat .
FROM alpine:3.18
RUN apk add --no-cache ca-certificates
WORKDIR /app
# Copy the binary, config directory, and www assets
COPY --from=builder /heartbeat/heartbeat /app/heartbeat
COPY --from=builder /heartbeat/config /app/config
COPY --from=builder /heartbeat/www /app/www
EXPOSE 8080
CMD ["/app/heartbeat"]There was a problem hiding this comment.
Okay I do actually need to test this on my end.
How does yours connect to DB?
There was a problem hiding this comment.
Could we have this as a post-commit hook or a CI lint instead?
There was a problem hiding this comment.
I like having the option to lint code myself before committing, and then ci only does a simple pass/fail but doesn't actively lint itself. does this make sense?
There was a problem hiding this comment.
Some of these seem kind of excessive like... detect-private-key?
Is this relevant to heartbeat or is that generic "oops the user committed a private key"?
And check-json + check-yaml aren't applicable as we don't have either of those.
Mixed LF is not strictly enforced but is going to be immediately noticable before committing (i.e., your diff is garbled).
Could we please cut this down to just the Go projects? Related, I haven't had formatter errors with qtpl templates I think we can just fmt them too.
I am unable to reproduce the goimports changes btw |
|---|
 |
| lint: | ||
| @echo "Running pre-commit hooks..." | ||
| @if command -v pre-commit >/dev/null 2>&1; then \ | ||
| pre-commit run --all-files; \ | ||
| else \ | ||
| echo "pre-commit not found. Install with: pip install pre-commit"; \ | ||
| exit 1; \ | ||
| fi |
There was a problem hiding this comment.
I do not want Python dependencies, can we convert whatever the pre-commit hook is supposed to be doing into a POSIX SH / Bash script?
I can write it, just let me know what it should be doing if not obvious.
There was a problem hiding this comment.
yeah totally doable tbh. pre-commit basically just runs linters/formatters on staged files before commit. the main sauce is:
- grab staged files w/ git diff --cached --name-only
- filter by extension (.go, .md, whatever)
- run your tools on those files as per the config file
- exit 1 if any fail
the tricky bit pre-commit handles is the stash dance - it stashes unstaged changes so you're ONLY checking what's actually being committed:
pfeifferj
changed the title
|
Oh shoot bump reminder to myself to do this |
| github.com/go-redis/redis/v8 v8.11.5 | ||
| github.com/joho/godotenv v1.5.1 | ||
| github.com/nitishm/go-rejson/v4 v4.2.0 | ||
| github.com/redis/go-redis/v9 v9.6.0 |
There was a problem hiding this comment.
Okay I do actually need to test this on my end.
How does yours connect to DB?
There was a problem hiding this comment.
Some of these seem kind of excessive like... detect-private-key?
Is this relevant to heartbeat or is that generic "oops the user committed a private key"?
And check-json + check-yaml aren't applicable as we don't have either of those.
Mixed LF is not strictly enforced but is going to be immediately noticable before committing (i.e., your diff is garbled).
Could we please cut this down to just the Go projects? Related, I haven't had formatter errors with qtpl templates I think we can just fmt them too.
I am unable to reproduce the goimports changes btw |
|---|
|
2 participants
adding pre-commit hooks and a make target to lint the codebase