Skip to content

Fix lint errors#100

Merged
pglass merged 8 commits intomainfrom
pglass/fix-lint-errors
Jun 24, 2025
Merged

Fix lint errors#100
pglass merged 8 commits intomainfrom
pglass/fix-lint-errors

Conversation

@pglass
Copy link
Copy Markdown
Contributor

@pglass pglass commented Jun 23, 2025

What was changed

  • Fix all lint errors
  • Enable golangci-lint in PR checks

Why?

Checklist

  1. Closes

  2. How was this tested:

  1. Any docs updates needed?

@pglass pglass requested a review from a team as a code owner June 23, 2025 17:39
@pglass pglass changed the title Pglass/fix lint errors Fix lint errors Jun 23, 2025
Base automatically changed from pglass/import-lint-fix to main June 23, 2025 21:56
@temporal-nick
Copy link
Copy Markdown
Collaborator

FYI, build failed:

go get -tool -modfile=develop/tools.mod golang.org/x/tools/cmd/goimports@latest
flag provided but not defined: -tool
usage: go get [-t] [-u] [-v] [build flags] [packages]
Run 'go help get' for details.
make: *** [Makefile:3[7](https://github.com/temporalio/s2s-proxy/actions/runs/15831199136/job/44624016012?pr=100#step:4:8): update-tools] Error 2

@pglass pglass force-pushed the pglass/fix-lint-errors branch from e708ce3 to 2e88d90 Compare June 24, 2025 15:36
Comment thread .golangci.yml
enable:
- gci
settings:
gci:
Copy link
Copy Markdown
Contributor Author

@pglass pglass Jun 24, 2025

Choose a reason for hiding this comment

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

Switched to https://github.com/daixiang0/gci for code formatting. I've seen a couple other teams use this. It does a better job of grouping imports than goimport does.

goimports doesn't merge two import groups if they are adjacent, and it allows import groups to be in any order

import (
	"github.com/hashicorp/yamux"
	"go.temporal.io/server/common/backoff"

	"github.com/temporalio/s2s-proxy/config" // <<<

	"github.com/temporalio/s2s-proxy/encryption" // <<<
	"github.com/temporalio/s2s-proxy/metrics"

	"fmt"
	"net"

While with gci, it will sort and merge into a minimum number of import groups:

import (
	"fmt"
	"net"

	"github.com/hashicorp/yamux"
	"go.temporal.io/server/common/backoff"

	"github.com/temporalio/s2s-proxy/config"
	"github.com/temporalio/s2s-proxy/encryption"
	"github.com/temporalio/s2s-proxy/metrics"

Comment thread Makefile
# Lint target
# Refer to .golangci.yml for configuration options
fmt:
$(GO_TOOL) golangci-lint fmt
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Switch to use golangci-lint fmt, which supports a few different standard formatters out of the box (so one fewer thing to install): https://golangci-lint.run/usage/formatters/#goimports

@pglass pglass merged commit 728518d into main Jun 24, 2025
8 checks passed
@pglass pglass deleted the pglass/fix-lint-errors branch June 24, 2025 17:09
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