cooperspencer / gickup

https://cooperspencer.github.io/gickup-documentation/
Apache License 2.0
962 stars 34 forks source link

chore: add linting #101

Closed tomMoulard closed 2 years ago

tomMoulard commented 2 years ago

Description

This PR adds linting to the project by using golangci-lint. It also adds a GitHub action that checks that PR that are proposed follow the same guidelines. Those guidelines can be changes in the .golangci.yml file.

This PR also adds a Makefile for easier local development.

WDYT ?

Edit: And thanks for this awesome tool :smile:

More information

Writing something like the following would be easier to read:

// logger is defined once, but used every time a log is needed.
logger := log.Str("stage", "foo").
    Str("url", "https://bar")

logger.Fatal().Msg(err.Error())
cooperspencer commented 2 years ago

Sounds like a good idea, thanks for that. I reviewed and ran it and everything seems alright, so I'll merge it.

Regarding the logging, your are right, I need to think about something there, because it really is cumbersome.

tomMoulard commented 2 years ago

Thanks for your review ! There is an issue with the golangci-lint action. I've tried to fix it, but it needs to be validated before I can check if the fix worked :sweat_smile:

cooperspencer commented 2 years ago

seems to work now. the only issue is the amd64 build for the docker container fails

cooperspencer commented 2 years ago

but that isn't because of your changes. I just re-ran the build on an older commit and it fails too. seems like something changed in the action.

tomMoulard commented 2 years ago

Yes, the builder docker layer in the Dockerfile is missing git to get dependencies.

I would suggest doing so:

FROM golang:alpine as builder

# Install dependencies for copy
RUN apk add -U --no-cache ca-certificates tzdata git

# Use an valid GOPATH and copy the files
WORKDIR /go/src/github.com/cooperspencer/gickup

COPY go.mod .
COPY go.sum .
RUN go mod tidy

COPY . .

# Fetching dependencies and build the app
RUN go get -d -v ./...
RUN CGO_ENABLED=0 go build -a -installsuffix cgo -o app .
cooperspencer commented 2 years ago

good idea, thanks