Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add golangci-lint Github Action formatter and lint checks #140

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

rsned
Copy link
Collaborator

@rsned rsned commented Apr 3, 2025

This adds a basic configuration that has gofmt and some basic lints enabled.

staticcheck is currently turned off because there are a number of outstanding code elements it flagged in the existing code that need to be fixed up before it should be turned on.

Copy link
Collaborator

@jmr jmr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will look more later. Was this copied from somewhere or based on some docs?

name: lint
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should always be pinned to a hash to avoid compromised deps.

https://blog.rafaelgss.dev/why-you-should-pin-actions-by-commit-hash

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. I filed #142 which, AFAIU, includes a check for this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you know where to find the hash to use?

https://github.com/actions/checkout/releases If i click on 4.2.2 the current latest release, I see
actions/checkout@11bd719 which I am guessing is the hash I want to use.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actions/checkout@11bd719 seems right to me. It's the last commit here:
actions/checkout@v4.2.1...v4.2.2

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@jmr jmr changed the title Add golangi-lint Github Action to enable formatter and lint checks. Add golangci-lint Github Action formatter and lint checks Apr 3, 2025
rsned and others added 3 commits April 3, 2025 16:23
Invalid Go toolchain version (1 result)
    * go.mod#L0C0:0: As of Go 1.21, toolchain versions [must use the 1.N.P syntax](https://go.dev/doc/toolchain#version).

      `1.21` in `go.mod` does not match this syntax and there is no additional `toolchain` directive, which may cause some `go` commands to fail.
(with comments on what the hash is representing
@rsned
Copy link
Collaborator Author

rsned commented Apr 8, 2025

The push failed because I wasn't sycned up. Now, for really real it is pushed.

Copy link
Collaborator

@alan-strohm alan-strohm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. @jmr are you still reviewing?

- master
pull_request:

permissions:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

settings:
gofmt:
# Simplify code: gofmt with `-s` option.
# Default: true
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not simplify if that's the default?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants