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

lint some of the things #4570

Merged
merged 15 commits into from
Jun 4, 2024
Merged

lint some of the things #4570

merged 15 commits into from
Jun 4, 2024

Conversation

mxyng
Copy link
Contributor

@mxyng mxyng commented May 22, 2024

now that ollama uses go1.22, x/exp/slices can be replaced with regular slices

enable some useful linters:

  • intrange is a 1.22 feature which simplifies for i := 0; i < n; i++ { } with for i := range n { }
  • testifylint to find bad testify assertions
  • unconvert to find unnecessary type conversions
  • usestdlibvars to find values that can be replaced with stdlib vars, e.g. OPTIONS with http.MethodOptions
  • wastedassign to find unnecessary assignments
  • whitespace is find unnecessary line

cmd/cmd.go Outdated Show resolved Hide resolved
@mxyng mxyng force-pushed the mxyng/slices branch 2 times, most recently from 4b975d2 to ecc8299 Compare May 22, 2024 16:05
@mxyng mxyng changed the title replace x/exp/slices with slices lint all the things May 22, 2024
@mxyng mxyng force-pushed the mxyng/slices branch 2 times, most recently from cf3c23d to a916ead Compare May 22, 2024 16:32
@mxyng mxyng changed the title lint all the things lint some of the things May 22, 2024
@@ -963,7 +962,7 @@ func allowedHostsMiddleware(addr net.Addr) gin.HandlerFunc {
}

if allowedHost(host) {
if c.Request.Method == "OPTIONS" {
if c.Request.Method == http.MethodOptions {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is far less clear. Let's turn this lint rule off.

From the maintainers of net/http, in summary: "This was a mistake to introduce these constants. It's less clear and readable using them."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm fine turning off the linter however I disagree it's less clear. The var is much easier to search for and less error prone since typos causes build errors.

@mxyng mxyng force-pushed the mxyng/slices branch 3 times, most recently from c89d08a to bc05f5f Compare May 30, 2024 01:13
.golangci.yaml Outdated Show resolved Hide resolved
cmd/start.go Show resolved Hide resolved
@mxyng mxyng force-pushed the mxyng/slices branch 4 times, most recently from 53c3831 to 17cf925 Compare June 4, 2024 20:19
@mxyng mxyng merged commit 89d9900 into main Jun 4, 2024
12 checks passed
@mxyng mxyng deleted the mxyng/slices branch June 4, 2024 20:27
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.

None yet

4 participants