chore: Add support for go1.23 and golangci-lint v1.60.1 #3101
Conversation
gaby
requested review from
sixcolors,
ReneWerner87 and
efectn
and removed request for
a team
|
Caution Review failedThe pull request is closed. WalkthroughThe recent changes primarily focus on updating the Fiber framework's documentation and configurations to reflect the new minimum required Go version of Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant Fiber
participant Go
User->>CLI: Install Fiber
CLI->>Fiber: Fetch latest version
Fiber->>Go: Check version compatibility
Go-->>Fiber: Confirm Go version >= 1.22
Fiber-->>CLI: Installation successful
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3101 +/- ##
==========================================
- Coverage 83.60% 80.84% -2.76%
==========================================
Files 115 116 +1
Lines 8342 8850 +508
==========================================
+ Hits 6974 7155 +181
- Misses 1042 1298 +256
- Partials 326 397 +71
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
go.modis excluded by!**/*.mod
Files selected for processing (32)
- .github/README.md (3 hunks)
- .github/workflows/benchmark.yml (1 hunks)
- .github/workflows/linter.yml (1 hunks)
- .github/workflows/test.yml (2 hunks)
- Makefile (2 hunks)
- client/client.go (1 hunks)
- client/client_test.go (1 hunks)
- client/request_test.go (2 hunks)
- ctx_test.go (3 hunks)
- docs/intro.md (1 hunks)
- docs/whats_new.md (1 hunks)
- helpers_test.go (2 hunks)
- listen.go (1 hunks)
- middleware/cache/cache_test.go (2 hunks)
- middleware/cache/manager.go (1 hunks)
- middleware/cache/manager_msgp.go (5 hunks)
- middleware/cache/manager_msgp_test.go (5 hunks)
- middleware/csrf/storage_manager.go (1 hunks)
- middleware/csrf/storage_manager_msgp.go (2 hunks)
- middleware/csrf/storage_manager_msgp_test.go (1 hunks)
- middleware/idempotency/response.go (1 hunks)
- middleware/idempotency/response_msgp.go (6 hunks)
- middleware/idempotency/response_msgp_test.go (5 hunks)
- middleware/limiter/manager.go (1 hunks)
- middleware/limiter/manager_msgp.go (1 hunks)
- middleware/limiter/manager_msgp_test.go (5 hunks)
- middleware/session/data.go (1 hunks)
- middleware/session/data_msgp.go (1 hunks)
- middleware/session/data_msgp_test.go (1 hunks)
- middleware/static/config.go (1 hunks)
- path_test.go (5 hunks)
- prefork_test.go (1 hunks)
Files skipped from review due to trivial changes (7)
- .github/README.md
- client/client.go
- helpers_test.go
- middleware/cache/cache_test.go
- middleware/limiter/manager.go
- middleware/session/data_msgp.go
- path_test.go
Additional context used
golangci-lint
prefork_test.go
98-98: directive
//nolint:tenv // Ignore erroris unused for linter "tenv"(nolintlint)
104-104: directive
//nolint:tenv // Ignore erroris unused for linter "tenv"(nolintlint)
listen.go
354-354: directive
//nolint:errcheck,revive,govet // ignore erroris unused for linter "govet"(nolintlint)
client/client_test.go
1425-1425: directive
//nolint: staticcheck // not needed for testsis unused for linter "staticcheck"(nolintlint)
client/request_test.go
87-87: directive
//nolint: staticcheck // not needed for testsis unused for linter "staticcheck"(nolintlint)
ctx_test.go
870-870: directive
//nolint: staticcheck // not needed for testsis unused for linter "staticcheck"(nolintlint)
883-883: directive
//nolint: staticcheck // not needed for testsis unused for linter "staticcheck"(nolintlint)
903-903: directive
//nolint: staticcheck // not needed for testsis unused for linter "staticcheck"(nolintlint)
GitHub Check: codecov/patch
middleware/cache/manager_msgp.go
[warning] 16-17: middleware/cache/manager_msgp.go#L16-L17
Added lines #L16 - L17 were not covered by tests
[warning] 23-24: middleware/cache/manager_msgp.go#L23-L24
Added lines #L23 - L24 were not covered by tests
[warning] 31-32: middleware/cache/manager_msgp.go#L31-L32
Added lines #L31 - L32 were not covered by tests
[warning] 37-38: middleware/cache/manager_msgp.go#L37-L38
Added lines #L37 - L38 were not covered by tests
[warning] 42-48: middleware/cache/manager_msgp.go#L42-L48
Added lines #L42 - L48 were not covered by tests
[warning] 50-53: middleware/cache/manager_msgp.go#L50-L53
Added lines #L50 - L53 were not covered by tests
[warning] 55-55: middleware/cache/manager_msgp.go#L55
Added line #L55 was not covered by tests
[warning] 60-61: middleware/cache/manager_msgp.go#L60-L61
Added lines #L60 - L61 were not covered by tests
[warning] 66-67: middleware/cache/manager_msgp.go#L66-L67
Added lines #L66 - L67 were not covered by tests
[warning] 72-73: middleware/cache/manager_msgp.go#L72-L73
Added lines #L72 - L73 were not covered by tests
[warning] 78-79: middleware/cache/manager_msgp.go#L78-L79
Added lines #L78 - L79 were not covered by tests
[warning] 84-85: middleware/cache/manager_msgp.go#L84-L85
Added lines #L84 - L85 were not covered by tests
[warning] 90-91: middleware/cache/manager_msgp.go#L90-L91
Added lines #L90 - L91 were not covered by tests
[warning] 93-97: middleware/cache/manager_msgp.go#L93-L97
Added lines #L93 - L97 were not covered by tests
[warning] 110-110: middleware/cache/manager_msgp.go#L110
Added line #L110 was not covered by tests
[warning] 114-115: middleware/cache/manager_msgp.go#L114-L115
Added lines #L114 - L115 were not covered by tests
[warning] 118-121: middleware/cache/manager_msgp.go#L118-L121
Added lines #L118 - L121 were not covered by tests
[warning] 123-126: middleware/cache/manager_msgp.go#L123-L126
Added lines #L123 - L126 were not covered by tests
[warning] 132-132: middleware/cache/manager_msgp.go#L132
Added line #L132 was not covered by tests
[warning] 136-137: middleware/cache/manager_msgp.go#L136-L137
Added lines #L136 - L137 were not covered by tests
[warning] 142-142: middleware/cache/manager_msgp.go#L142
Added line #L142 was not covered by tests
[warning] 146-147: middleware/cache/manager_msgp.go#L146-L147
Added lines #L146 - L147 were not covered by tests
[warning] 152-152: middleware/cache/manager_msgp.go#L152
Added line #L152 was not covered by tests
[warning] 156-157: middleware/cache/manager_msgp.go#L156-L157
Added lines #L156 - L157 were not covered by tests
[warning] 162-162: middleware/cache/manager_msgp.go#L162
Added line #L162 was not covered by tests
[warning] 166-167: middleware/cache/manager_msgp.go#L166-L167
Added lines #L166 - L167 were not covered by tests
[warning] 172-172: middleware/cache/manager_msgp.go#L172
Added line #L172 was not covered by tests
[warning] 176-177: middleware/cache/manager_msgp.go#L176-L177
Added lines #L176 - L177 were not covered by tests
[warning] 182-182: middleware/cache/manager_msgp.go#L182
Added line #L182 was not covered by tests
[warning] 186-187: middleware/cache/manager_msgp.go#L186-L187
Added lines #L186 - L187 were not covered by tests
Additional comments not posted (69)
middleware/idempotency/response.go (1)
6-6: LGTM! Enhanced testing capabilities.The addition of the
-tests=trueflag in thego:generatedirective improves the testing capabilities of the generated code.middleware/session/data.go (2)
7-8: LGTM! Improved code generation setup.The
go:generatedirective has been updated to include tests, enhancing the testing capabilities of the generated code.
12-12: LGTM! Proper exclusion ofsync.RWMutex.The addition of the
msg:"-"tag tosync.RWMutexensures it is excluded from serialization, which is a best practice..github/workflows/linter.yml (2)
33-33: LGTM! Updated Go version.The Go version has been updated to
1.22.x, ensuring compatibility with newer features.
40-40: LGTM! Updated golangci-lint version.The golangci-lint version has been updated to
v1.60.1, incorporating improvements and bug fixes..github/workflows/test.yml (1)
18-18: LGTM! Updates to Go versions and coverage upload conditions are appropriate.The changes to the Go version matrix and the conditional statement for uploading coverage reports align with the PR objectives of supporting Go 1.23.
Also applies to: 34-34
middleware/csrf/storage_manager.go (2)
12-12: LGTM! Updatedgo:generatedirective is correct.The change from
// go:generate msgpto the specific command ensures proper code generation.
19-21: LGTM! Struct field annotations and linter comments are appropriate.The
msg:"-"annotations correctly exclude fields from serialization, and thenolint:revivecomments suppress warnings for unexported types.Makefile (2)
38-38: LGTM! Linter version update is appropriate.Updating to
v1.60.1ensures the latest linter features and fixes are applied.
60-67: LGTM! Newmsgptarget is well-defined.The
msgptarget facilitates message pack generation for multiple files, improving serialization processes.middleware/static/config.go (1)
89-89: Verifycfg.IndexNamesis nevernil.The condition now assumes
cfg.IndexNameswill not benil. Ensure that the rest of the codebase guarantees this to prevent potentialnilpointer dereferences.Verification successful
cfg.IndexNamesis nevernil.The
IndexNamesfield in theConfigstruct is initialized with a default value of[]string{"index.html"}, ensuring it is notnil. Additionally, there are no assignments in the codebase that setIndexNamestonil. Thus, the conditionif len(cfg.IndexNames) == 0is safe and does not risk anilpointer dereference.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that `cfg.IndexNames` is never `nil` before calling `configDefault`. # Test: Search for instances where `configDefault` is called. Expect: `cfg.IndexNames` is initialized. rg --type go -A 5 $'configDefault'Length of output: 16218
Script:
#!/bin/bash # Search for the definition of the `Config` struct to check how `IndexNames` is initialized. ast-grep --lang go --pattern 'type Config struct { $$$ }'Length of output: 71614
Script:
#!/bin/bash # Search for assignments to `cfg.IndexNames` to ensure it is not set to `nil`. rg --type go 'IndexNames\s*=\s*nil'Length of output: 37
middleware/cache/manager_msgp_test.go (1)
Line range hint
12-92:
LGTM! Improved clarity and test coverage.The renaming of functions for clarity and the addition of new test and benchmark functions enhance the code quality and functionality.
Also applies to: 94-123
middleware/limiter/manager_msgp_test.go (7)
Line range hint
12-34: Review ofTestMarshalUnmarshalitemfunction.The function tests the marshal and unmarshal operations for the
itemstruct. The renaming improves clarity. The test logic is sound, ensuring no leftover bytes after unmarshalling and skipping.
Line range hint
35-43: Review ofBenchmarkMarshalMsgitemfunction.The benchmark measures the performance of the
MarshalMsgmethod for theitemstruct. The renaming improves clarity. The function is correctly set up to report allocations and reset timers.
Line range hint
44-55: Review ofBenchmarkAppendMsgitemfunction.This benchmark tests the performance of appending a marshalled message. The renaming improves clarity. The setup correctly handles byte slices and reports allocations.
Line range hint
56-68: Review ofBenchmarkUnmarshalitemfunction.The benchmark evaluates the performance of the
UnmarshalMsgmethod. The renaming improves clarity. The function correctly handles error checking and reports allocations.
70-92: Review ofTestEncodeDecodeitemfunction.The new test function enhances coverage by testing encoding and decoding operations. It checks the accuracy of
Msgsize()and ensures no errors during decoding. The warning log is useful for debugging.
94-106: Review ofBenchmarkEncodeitemfunction.This new benchmark measures the performance of encoding operations. The setup is correct with buffer management and allocation reporting. The function is well-structured for performance testing.
108-123: Review ofBenchmarkDecodeitemfunction.The benchmark tests decoding performance. The setup is correct with buffer management and allocation reporting. The function is well-structured and handles errors appropriately.
middleware/idempotency/response_msgp_test.go (6)
Line range hint
35-68: Review ofBenchmarkMarshalMsgresponsefunction.The benchmark measures the performance of the
MarshalMsgmethod for theresponsestruct. The renaming improves clarity. The function is correctly set up to report allocations and reset timers.
Line range hint
44-55: Review ofBenchmarkAppendMsgresponsefunction.This benchmark tests the performance of appending a marshalled message. The renaming improves clarity. The setup correctly handles byte slices and reports allocations.
Line range hint
56-68: Review ofBenchmarkUnmarshalresponsefunction.The benchmark evaluates the performance of the
UnmarshalMsgmethod. The renaming improves clarity. The function correctly handles error checking and reports allocations.
70-92: Review ofTestEncodeDecoderesponsefunction.The new test function enhances coverage by testing encoding and decoding operations. It checks the accuracy of
Msgsize()and ensures no errors during decoding. The warning log is useful for debugging.
94-106: Review ofBenchmarkEncoderesponsefunction.This new benchmark measures the performance of encoding operations. The setup is correct with buffer management and allocation reporting. The function is well-structured for performance testing.
108-123: Review ofBenchmarkDecoderesponsefunction.The benchmark tests decoding performance. The setup is correct with buffer management and allocation reporting. The function is well-structured and handles errors appropriately.
middleware/session/data_msgp_test.go (7)
12-33: Review ofTestMarshalUnmarshaldatafunction.The function tests the marshal and unmarshal operations for the
datastruct. It ensures no leftover bytes after unmarshalling and skipping. The logic is sound and comprehensive.
35-42: Review ofBenchmarkMarshalMsgdatafunction.The benchmark measures the performance of the
MarshalMsgmethod for thedatastruct. The setup is correct with allocation reporting and timer resetting.
44-54: Review ofBenchmarkAppendMsgdatafunction.This benchmark tests the performance of appending a marshalled message. The setup correctly handles byte slices and reports allocations.
56-68: Review ofBenchmarkUnmarshaldatafunction.The benchmark evaluates the performance of the
UnmarshalMsgmethod. The function correctly handles error checking and reports allocations.
70-92: Review ofTestEncodeDecodedatafunction.The test function enhances coverage by testing encoding and decoding operations. It checks the accuracy of
Msgsize()and ensures no errors during decoding. The warning log is useful for debugging.
94-106: Review ofBenchmarkEncodedatafunction.This benchmark measures the performance of encoding operations. The setup is correct with buffer management and allocation reporting. The function is well-structured for performance testing.
108-123: Review ofBenchmarkDecodedatafunction.The benchmark tests decoding performance. The setup is correct with buffer management and allocation reporting. The function is well-structured and handles errors appropriately.
middleware/cache/manager.go (1)
11-13: Enabling test generation formsgpis a positive change.The modification to the
msgpdirective to generate tests will enhance the reliability and maintainability of the generated code.middleware/limiter/manager_msgp.go (1)
9-90: The implementation ofDecodeMsgandEncodeMsgenhances serialization capabilities.The new methods for encoding and decoding
iteminstances using MessagePack format improve the structure's functionality. The error handling is robust and provides clear context, which is beneficial for debugging..github/workflows/benchmark.yml (1)
34-34: Updating Go version to1.22.xis a beneficial change.The update to the Go version in the GitHub Actions workflow ensures that the project benefits from the latest features and improvements available in Go
1.22.x.middleware/csrf/storage_manager_msgp.go (9)
9-35: Verify handling of map keys inDecodeMsg.The
DecodeMsgmethod currently defaults to skipping all keys. Ensure that this is intentional and that no fields need to be decoded.
38-47: LGTM!The
EncodeMsgmethod correctly appends a map header with size 0 for an empty struct.
48-53: LGTM!The
MarshalMsgmethod is consistent withEncodeMsgand correctly handles an empty struct.
Line range hint
54-92:
Verify handling of map keys inUnmarshalMsg.The
UnmarshalMsgmethod currently skips all keys. Ensure that this is intentional and that no fields need to be unmarshaled.
93-93: LGTM!The
Msgsizemethod correctly returns 1 for an empty struct.
94-120: Verify handling of map keys inDecodeMsg.The
DecodeMsgmethod currently skips all keys. Ensure that this is intentional and that no fields need to be decoded forstorageManager.
123-132: LGTM!The
EncodeMsgmethod correctly appends a map header with size 0 for an empty struct.
143-171: Verify handling of map keys inUnmarshalMsg.The
UnmarshalMsgmethod currently skips all keys. Ensure that this is intentional and that no fields need to be unmarshaled forstorageManager.
173-177: LGTM!The
Msgsizemethod correctly returns 1 for an empty struct.docs/intro.md (1)
15-15: LGTM!The Go version requirement update to
1.22is consistent with the PR objectives.middleware/csrf/storage_manager_msgp_test.go (14)
12-33: LGTM!The
TestMarshalUnmarshalitemfunction correctly tests the marshal and unmarshal operations foritem.
35-42: LGTM!The
BenchmarkMarshalMsgitemfunction correctly benchmarks theMarshalMsgoperation foritem.
44-54: LGTM!The
BenchmarkAppendMsgitemfunction correctly benchmarks the append operation foritem.
56-68: LGTM!The
BenchmarkUnmarshalitemfunction correctly benchmarks theUnmarshalMsgoperation foritem.
70-92: LGTM!The
TestEncodeDecodeitemfunction correctly tests the encode and decode operations foritem.
94-106: LGTM!The
BenchmarkEncodeitemfunction correctly benchmarks theEncodeMsgoperation foritem.
108-123: LGTM!The
BenchmarkDecodeitemfunction correctly benchmarks theDecodeMsgoperation foritem.
125-146: LGTM!The
TestMarshalUnmarshalstorageManagerfunction correctly tests the marshal and unmarshal operations forstorageManager.
148-155: LGTM!The
BenchmarkMarshalMsgstorageManagerfunction correctly benchmarks theMarshalMsgoperation forstorageManager.
157-167: LGTM!The
BenchmarkAppendMsgstorageManagerfunction correctly benchmarks the append operation forstorageManager.
169-181: LGTM!The
BenchmarkUnmarshalstorageManagerfunction correctly benchmarks theUnmarshalMsgoperation forstorageManager.
183-205: LGTM!The
TestEncodeDecodestorageManagerfunction correctly tests the encode and decode operations forstorageManager.
207-219: LGTM!The
BenchmarkEncodestorageManagerfunction correctly benchmarks theEncodeMsgoperation forstorageManager.
221-236: LGTM!The
BenchmarkDecodestorageManagerfunction correctly benchmarks theDecodeMsgoperation forstorageManager.middleware/idempotency/response_msgp.go (5)
9-91: Enhancements toDecodeMsglook good.The function now correctly handles additional fields and improves error handling by wrapping errors with contextual information.
93-145: Enhancements toEncodeMsglook good.The function now correctly encodes additional fields and improves error handling by wrapping errors with contextual information.
Line range hint
147-167:
Update toMarshalMsgaligns with new encoding structure.The function now appends
StatusCodeat the end, ensuring consistency with the updated encoding structure.
Line range hint
169-243:
Update toUnmarshalMsgensures complete field processing.The function now includes the
StatusCodefield, ensuring all fields are decoded correctly with proper error handling.
Line range hint
258-269:
Update toMsgsizeensures accurate size estimation.The function now accounts for the new encoding structure, ensuring the estimated size reflects the actual size accurately.
middleware/cache/manager_msgp.go (3)
191-202: Update toMarshalMsgaligns with encoding structure.The function correctly marshals fields into a byte slice, consistent with the
EncodeMsgstructure.
Line range hint
204-301:
Update toUnmarshalMsgaligns with decoding structure.The function correctly unmarshals fields from a byte slice, consistent with the
DecodeMsgstructure.
322-330: Update toMsgsizeensures accurate size estimation.The function now accounts for the new encoding structure, ensuring the estimated size reflects the actual size accurately.
docs/whats_new.md (1)
41-41: Documentation Update Approved.The update correctly reflects the new minimum Go version requirement for Fiber v3.
client/request_test.go (1)
1606-1607: Approved: Simplification of assertions.The simplification of assertions in the benchmark enhances readability without altering functionality.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.50.
| Benchmark suite | Current: 05ac0d0 | Previous: e437633 | Ratio |
|---|---|---|---|
Benchmark_Compress_Levels/Brotli_LevelBestCompression - B/op |
5 B/op |
0 B/op |
+∞ |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Actionable comments posted: 11
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- .github/workflows/test.yml (2 hunks)
- middleware/limiter/limiter_fixed.go (3 hunks)
- middleware/limiter/limiter_sliding.go (2 hunks)
- middleware/limiter/limiter_test.go (2 hunks)
- prefork.go (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- .github/workflows/test.yml
Additional context used
golangci-lint
middleware/limiter/limiter_fixed.go
[warning] 30-30: var-naming: don't use underscores in Go names; var max_requests should be maxRequests
(revive)
30-30: ST1003: should not use underscores in Go names; var max_requests should be maxRequests
(stylecheck)
middleware/limiter/limiter_sliding.go
[warning] 31-31: var-naming: don't use underscores in Go names; var max_requests should be maxRequests
(revive)
31-31: ST1003: should not use underscores in Go names; var max_requests should be maxRequests
(stylecheck)
prefork.go
79-79: ST1003: should not use underscores in Go names; var max_procs should be maxProcs
(stylecheck)
[warning] 79-79: var-naming: don't use underscores in Go names; var max_procs should be maxProcs
(revive)
middleware/limiter/limiter_test.go
[warning] 100-100: var-naming: don't use underscores in Go names; var mas_requests should be masRequests
(revive)
100-100: ST1003: should not use underscores in Go names; var mas_requests should be masRequests
(stylecheck)
GitHub Check: lint
middleware/limiter/limiter_fixed.go
[failure] 30-30:
var-naming: don't use underscores in Go names; var max_requests should be maxRequests (revive)
[failure] 30-30:
ST1003: should not use underscores in Go names; var max_requests should be maxRequests (stylecheck)middleware/limiter/limiter_sliding.go
[failure] 31-31:
var-naming: don't use underscores in Go names; var max_requests should be maxRequests (revive)
[failure] 31-31:
ST1003: should not use underscores in Go names; var max_requests should be maxRequests (stylecheck)prefork.go
[failure] 79-79:
var-naming: don't use underscores in Go names; var max_procs should be maxProcs (revive)
[failure] 79-79:
ST1003: should not use underscores in Go names; var max_procs should be maxProcs (stylecheck)middleware/limiter/limiter_test.go
[failure] 100-100:
var-naming: don't use underscores in Go names; var mas_requests should be masRequests (revive)
[failure] 100-100:
ST1003: should not use underscores in Go names; var mas_requests should be masRequests (stylecheck)
Description
msgpcommand to Makefilemsgpfiles because of the following:// go:generate msgp, this is broken//go:generate msgp, this is correctType of change