chore: use errors.New to replace fmt.Errorf with no parameters #1013
Conversation
Signed-off-by: ChengenH <hce19970702@gmail.com>
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 golangci-lint (1.62.2)level=warning msg="The linter 'exportloopref' is deprecated (since v1.60.2) due to: Since Go1.22 (loopvar) this linter is no longer relevant. Replaced by copyloopvar." WalkthroughThe pull request primarily focuses on enhancing error handling across multiple files by replacing instances of Changes
Possibly related PRs
Suggested reviewers
Poem
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
v2/migrate/v0/migrate_v0.go (1)
Empty if block needs implementation for setting the latest version
Based on the code context, this is a command that handles both getting and setting the latest version in the metadata.sqlite database. While the 'get' functionality is implemented in the else block, the 'set' functionality is completely missing in the if block. This confirms the original review comment about the empty if block being an incomplete implementation.
The empty block should contain logic to:
- Serialize the version number to protobuf format
- Store it in the database using the
latestVersionKey🔗 Analysis chain
Line range hint
118-119: Empty if block needs implementation.The
if set { }block is empty, which suggests incomplete implementation. This could lead to silent failures when attempting to set the version.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if there are any TODOs or related implementation notes rg -i "todo.*latest.*version|implement.*set.*version"Length of output: 55
Script:
#!/bin/bash # Let's check the surrounding context of the empty if block rg -B 5 -A 5 "if set \{" v2/migrate/v0/migrate_v0.goLength of output: 262
Script:
#!/bin/bash # Let's also check for any similar patterns or implementations in the codebase ast-grep --pattern 'if set { $$$ }'Length of output: 765
Script:
#!/bin/bash # Let's see the entire function implementation for better context ast-grep --pattern 'func LatestVersion($$$) { $$$ }'Length of output: 56
Script:
#!/bin/bash # Let's check the function name and its full implementation rg -B 10 -A 20 "func.*Version.*\(" v2/migrate/v0/migrate_v0.goLength of output: 820
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (11)
cmd/iaviewer/main.go(2 hunks)mutable_tree.go(2 hunks)nodedb.go(1 hunks)proof.go(1 hunks)proof_ics23.go(3 hunks)v2/export.go(2 hunks)v2/migrate/v0/migrate_v0.go(2 hunks)v2/node.go(3 hunks)v2/sqlite.go(2 hunks)v2/sqlite_metadata.go(2 hunks)v2/tree.go(2 hunks)
🧰 Additional context used
🪛 golangci-lint (1.62.2)
proof_ics23.go
5-5: File is not gofumpt-ed
(gofumpt)
🔇 Additional comments (14)
v2/export.go (1)
3-5: LGTM: Appropriate use of errors.New
The change from fmt.Errorf to errors.New is correct here since the error message doesn't require any formatting.
Also applies to: 103-103
v2/sqlite_metadata.go (1)
4-4: LGTM: Appropriate use of errors.New
The change from fmt.Errorf to errors.New is correct here since the error message doesn't require any formatting.
Also applies to: 23-23
cmd/iaviewer/main.go (1)
7-7: LGTM: Appropriate use of errors.New
The change from fmt.Errorf to errors.New is correct here since the error message doesn't require any formatting. The remaining fmt.Errorf usage in the file is correctly left unchanged as it requires string formatting.
Also applies to: 71-71
proof.go (1)
22-28: LGTM! Good improvement in error handling.
The replacement of fmt.Errorf with errors.New for static error messages is appropriate here. This change:
- Improves performance by avoiding unnecessary string formatting
- Follows Go best practices for declaring constant error values
proof_ics23.go (1)
50-50: LGTM! Appropriate use of errors.New.
The changes correctly replace fmt.Errorf with errors.New for static error messages, improving code clarity and performance.
Also applies to: 179-179
v2/migrate/v0/migrate_v0.go (1)
115-115: LGTM! Good error handling improvement.
The replacement of fmt.Errorf with errors.New for the static error message is appropriate.
v2/node.go (3)
105-105: LGTM! Good simplification of error handling
The change from fmt.Errorf to errors.New is appropriate here since the error message is static and requires no formatting.
120-120: LGTM! Consistent error handling
The change to errors.New matches the similar change in getLeftNode, maintaining consistency in error handling throughout the codebase.
161-161: LGTM! Simplified error creation
The change to errors.New is appropriate for this static error message when attempting to balance a persisted node.
v2/tree.go (1)
106-106: LGTM! Appropriate error handling
The change to errors.New is suitable for this nil check error message.
v2/sqlite.go (1)
103-103: LGTM! Simplified error message
The change to errors.New is appropriate for this static "no row" error message.
mutable_tree.go (2)
963-963: LGTM: Good simplification of error handling
The change from fmt.Errorf to errors.New is appropriate here since the error message is static and doesn't require any string formatting.
1087-1087: LGTM: Appropriate error handling simplification
The change from fmt.Errorf to errors.New is correct here as the error message is static and doesn't require string interpolation.
nodedb.go (1)
363-363: LGTM: Simplified error handling
The change from fmt.Errorf to errors.New is appropriate here since the error message is static and doesn't require any string formatting.
No description provided.