enable additional linters #589
Conversation
|
@webmaster128 please check this out, as well as the other, related pull requests. |
There was a problem hiding this comment.
Not a full review.
I am interested in the linters (and the fixes that make them happy), but there are lots of unrelated changes in here that need to be discussed. I would appreciate if we keep these in separate PRs, so they don't block the us from merging the linters.
| if err != nil && err.(syscall.Errno) != C.ErrnoValue_Success { | ||
| if err != nil && err.(syscall.Errno) != 0 { |
There was a problem hiding this comment.
Why this change? I like the constant here because it makes it clear immediately that this is the unsuccessful case.
| @@ -1,6 +1,6 @@ | |||
| module github.com/CosmWasm/wasmvm/v2 | |||
|
|
|||
| go 1.21 | |||
| go 1.23 | |||
There was a problem hiding this comment.
Is this required for the linters?
| testMutex.Lock() | ||
| info = MockInfoBin(b, "fred") | ||
| testMutex.Unlock() |
There was a problem hiding this comment.
Sir, to understand why, please simply take the configuration file for the linter, put it in the main Branch, and you'll see why
There was a problem hiding this comment.
I ran golangci-lint run with the .golangci.yml from this branch and see no output related to this. I also ran it directly on this branch after removing the lock and it's happy without it.
It makes zero sense to lock this mock function call.
| // There is no allocation we can copy | ||
| out = []byte{} | ||
| } else { | ||
| // C.GoBytes create a copy (https://stackoverflow.com/a/40950744/2013738) |
There was a problem hiding this comment.
This removes an important comment
| @@ -92,7 +94,7 @@ func copyU8Slice(view C.U8SliceView) []byte { | |||
| // In this case, we don't want to look into the ptr | |||
| return []byte{} | |||
| } | |||
| // C.GoBytes create a copy (https://stackoverflow.com/a/40950744/2013738) | |||
This pull request adds additional linters and works towards a more ideal configuration for the linter.
This is the most completely linted version. Before reviewing this, I do recommend that you review The pull requests that are in the style requested by @webmaster128
He asked for multiple PRS, each enabling more linters. This is the last of them.