replace interface{} with any
#1560
Conversation
WalkthroughThe overall change focuses on updating the type system within the codebase, specifically transitioning from using Changes
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 Configration File (
|
|
Although this is not a bugfix, I will backport this into 1.8. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (8)
- README.md (1 hunks)
- driver_test.go (12 hunks)
- errors.go (1 hunks)
- fields.go (1 hunks)
- nulltime.go (1 hunks)
- nulltime_test.go (1 hunks)
- statement.go (1 hunks)
- statement_test.go (2 hunks)
Additional comments: 9
nulltime_test.go (1)
- 26-26: The change from
interface{}toanyaligns with the PR's goal of adopting Go 1.18'sanytype for improved code clarity. It's a good practice to add a brief comment explaining the use ofanyoverinterface{}for those who might not be familiar with this update in Go 1.18.nulltime.go (1)
- 41-41: The update to use
anyin theScanmethod's parameter type is a good improvement for code readability, aligning with the PR's objectives. Consider adding a comment about the use ofanyfor those unfamiliar with this Go 1.18 update.errors.go (2)
- 44-44: The update to use
anyin theLoggerinterface'sanyfor those unfamiliar with this Go 1.18 update.- 51-51: Similarly, the update in the
NopLoggerstruct'sanyis a good improvement for code readability. Adding a brief comment about the use ofanycould be helpful for clarity.statement_test.go (2)
- 80-80: The update to use
anyin theTestConvertSignedIntegersfunction enhances code readability and aligns with the PR's objectives. Consider adding a comment about the use ofanyfor those unfamiliar with this Go 1.18 update.- 109-109: Similarly, the update in the
TestConvertUnsignedIntegersfunction to useanyis a good improvement for code readability. Adding a brief comment about the use ofanycould be helpful for clarity.fields.go (1)
- 137-137: The update in the
scanTypeUnknowndeclaration to useanyenhances code readability and aligns with the PR's objectives. Consider adding a comment about the use ofanyfor those unfamiliar with this Go 1.18 update.statement.go (1)
- 144-144: The update to use
anyin theConvertValuemethod's parameter type is a good improvement for code readability, aligning with the PR's objectives. Consider adding a comment about the use ofanyfor those unfamiliar with this Go 1.18 update.README.md (1)
- 329-329: The change from
interface{}toanyin the function signature within the README.md is consistent with the PR's objectives to leverage Go 1.18'sanytype for better readability. This update should make the example code clearer to readers.
| dbt.Fatalf("error on %s %s: %s", method, query, err.Error()) | ||
| } | ||
|
|
||
| func (dbt *DBTest) mustExec(query string, args ...interface{}) (res sql.Result) { | ||
| func (dbt *DBTest) mustExec(query string, args ...any) (res sql.Result) { | ||
| dbt.Helper() | ||
| res, err := dbt.db.Exec(query, args...) | ||
| if err != nil { |
There was a problem hiding this comment.
📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1494-1496]
The TLS configuration is missing the MinVersion field. It's a good practice to explicitly set the minimum version of TLS to TLS 1.3 to ensure the use of secure protocols. Consider adding MinVersion: tls.VersionTLS13 to the TLS configuration.
tls.Config{
InsecureSkipVerify: true,
+ MinVersion: tls.VersionTLS13,
}|
|
||
| ```go | ||
| conn, _ := db.Conn(ctx) | ||
| conn.Raw(func(conn interface{}) error { | ||
| conn.Raw(func(conn any) error { | ||
| ex := conn.(driver.Execer) | ||
| res, err := ex.Exec(` | ||
| UPDATE point SET x = 1 WHERE y = 2; |
There was a problem hiding this comment.
📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [3-3]
There's a possible spelling mistake with "MySQL-Driver". Consider specifying it as "MySQL driver" for clarity and consistency with common terminology.
- A MySQL-Driver for Go's [database/sql](https://golang.org/pkg/database/sql/) package
+ A MySQL driver for Go's [database/sql](https://golang.org/pkg/database/sql/) package
There was a problem hiding this comment.
@coderabbitai If this is "outside the diff hunks", why are you posting this here? This is just irrelevant.
There was a problem hiding this comment.
@dolmen, thank you for your feedback. You're correct that the comment was outside the modified lines in the pull request, and I understand your concern about the relevance of such comments to the immediate changes.
The automated review process is designed to ensure overall quality and consistency throughout the file, which sometimes includes suggestions on parts of the file not directly modified but still visible in the broader context of the pull request. However, I appreciate that the primary focus should be on the changes made in the diff hunks.
If you prefer, I can adjust the settings to limit comments to only the lines modified in the pull request. Would you like me to make this change for future reviews?
There was a problem hiding this comment.
@dolmen See https://github.com/apps/coderabbitai
I just trying it. It is far from perfect, but interesting enough.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
Description
Go 1.18 introduced
any. Let's use it.Checklist
Summary by CodeRabbit
anytype instead ofinterface{}.