chore: cleanup xxx ptr to use lo#1238
Conversation
There was a problem hiding this comment.
Code Review
This pull request standardizes the creation of pointers in test files by replacing various custom helper functions with the lo.ToPtr utility from the samber/lo library. Several critical compilation errors were identified in channel_llm_codex_oauth_test.go and performance_test.go where the built-in new function was incorrectly used with values (e.g., new("hi"), new(true)) instead of types. These instances must be corrected to use lo.ToPtr to ensure the code compiles and aligns with the rest of the changes.
| Model: "gpt-4o-mini", | ||
| Messages: []llm.Message{ | ||
| {Role: "user", Content: llm.MessageContent{Content: loPtr("hi")}}, | ||
| {Role: "user", Content: llm.MessageContent{Content: new("hi")}}, |
There was a problem hiding this comment.
The new built-in function in Go takes a type as an argument (e.g., new(string)), not a value. Passing a string literal like "hi" will cause a compilation error. Since the goal of this PR is to use the lo library, you should use lo.ToPtr("hi") instead. Note that you will also need to add "github.com/samber/lo" to the imports in this file.
| {Role: "user", Content: llm.MessageContent{Content: new("hi")}}, | |
| {Role: "user", Content: llm.MessageContent{Content: lo.ToPtr("hi")}}, |
| { | ||
| name: "streaming request - Stream is true", | ||
| streamValue: boolPtr(true), | ||
| streamValue: new(true), |
There was a problem hiding this comment.
| { | ||
| name: "non-streaming request - Stream is false", | ||
| streamValue: boolPtr(false), | ||
| streamValue: new(false), |
| Model: "gpt-4o-mini", | ||
| Messages: []llm.Message{ | ||
| {Role: "user", Content: llm.MessageContent{Content: loPtr("hi")}}, | ||
| {Role: "user", Content: llm.MessageContent{Content: new("hi")}}, |
There was a problem hiding this comment.
🔴 new("hi") used instead of lo.ToPtr("hi") — Go builtin new takes a type, not a value
The Go builtin new function takes a type as its argument (e.g., new(string) returns *string pointing to ""). Passing a value like "hi" is invalid Go syntax and won't compile. This also violates the backend rule in .agent/rules/backend.md:72: "Use lo.ToPtr to get a pointer to a constant value instead of implementing helper functions like stringPtr." The correct replacement for the removed loPtr("hi") should be lo.ToPtr("hi").
Prompt for agents
In internal/server/biz/channel_llm_codex_oauth_test.go, line 67, replace `new("hi")` with `lo.ToPtr("hi")`. Also add `"github.com/samber/lo"` to the import block at the top of the file (around line 3-22).
Was this helpful? React with 👍 or 👎 to provide feedback.
| { | ||
| name: "streaming request - Stream is true", | ||
| streamValue: boolPtr(true), | ||
| streamValue: new(true), |
There was a problem hiding this comment.
🔴 new(true) used instead of lo.ToPtr(true) — Go builtin new takes a type, not a value
The Go builtin new function takes a type as its argument (e.g., new(bool) returns *bool pointing to false). Passing the value true is invalid Go syntax and won't compile. This also violates the backend rule in .agent/rules/backend.md:72: "Use lo.ToPtr to get a pointer to a constant value instead of implementing helper functions like stringPtr." The correct replacement for the removed boolPtr(true) should be lo.ToPtr(true).
Prompt for agents
In internal/server/orchestrator/performance_test.go, replace `new(true)` on line 33 with `lo.ToPtr(true)`, and `new(false)` on line 38 with `lo.ToPtr(false)`. Also add `"github.com/samber/lo"` to the import block at the top of the file (around lines 3-14).
Was this helpful? React with 👍 or 👎 to provide feedback.
| { | ||
| name: "non-streaming request - Stream is false", | ||
| streamValue: boolPtr(false), | ||
| streamValue: new(false), |
There was a problem hiding this comment.
🔴 new(false) used instead of lo.ToPtr(false) — Go builtin new takes a type, not a value
Same issue as new(true) on line 33. The Go builtin new takes a type, not a value — new(false) is invalid Go. This violates the backend rule in .agent/rules/backend.md:72 to use lo.ToPtr. The correct replacement for the removed boolPtr(false) should be lo.ToPtr(false).
| streamValue: new(false), | |
| streamValue: lo.ToPtr(false), |
Was this helpful? React with 👍 or 👎 to provide feedback.
Uh oh!
There was an error while loading. Please reload this page.