Conversation
extemporalgenome
force-pushed
the
kgillette/2025-07/lint-fixes
branch
2 times, most recently
from
f5aeae2 to
f2f526a
Compare
extemporalgenome
force-pushed
the
kgillette/2025-07/lint-fixes
branch
from
5a6f565 to
b5cb64d
Compare
modern go tooling requires a version with `go install`.
The constants in particular could have been used, but do not seem that they'd improve readability: these aren't "magic" byte literals, because they're well and widely understood aspects of the JSON syntax.
extemporalgenome
force-pushed
the
kgillette/2025-07/lint-fixes
branch
from
c6c371b to
0e2637e
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR addresses several golangci-lint issues to improve code quality and maintainability. The changes focus on removing unused code, updating deprecated API usage, and following Go best practices.
- Removes unused constants and variables
- Updates deprecated
reflect.PtrTocalls toreflect.PointerTo - Improves error handling with
errors.Is()and proper variable scoping
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| json/parse.go | Removes unused escape and quote constants |
| json/json_test.go | Adds lint suppression comments and improves error handling |
| json/json.go | Adds lint suppression comments for deprecated types |
| json/decode.go | Updates deprecated reflect API and marks unused parameter |
| json/codec.go | Updates deprecated reflect API and uses maps.Copy |
| benchmarks/Makefile | Pins benchstat installation to latest version |
| .golangci.yml | Adds linter configuration |
| .github/workflows/benchmark.yml | Fixes typo in step name |
|
|
||
| func cacheStore(typ reflect.Type, cod codec, oldCodecs map[unsafe.Pointer]codec) { | ||
| newCodecs := make(map[unsafe.Pointer]codec, len(oldCodecs)+1) | ||
| maps.Copy(newCodecs, oldCodecs) |
There was a problem hiding this comment.
Using maps.Copy is more efficient than manually iterating over the map entries
| for { | ||
| var o obj | ||
| err = d.Decode(&o) | ||
| err := d.Decode(&o) |
There was a problem hiding this comment.
Variable scoping improved by declaring err within the loop scope where it's used
| err := d.Decode(&o) | ||
| if err != nil { | ||
| if err == io.EOF { | ||
| if errors.Is(err, io.EOF) { |
There was a problem hiding this comment.
Using errors.Is() is the recommended way to check for specific errors instead of direct equality comparison
nainya
approved these changes
Jul 28, 2025
viveksainanee
approved these changes
Jul 28, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Several (but not all) golangci-lint issues have been resolved.
Benchmark impact appears to be neutral.