Repository navigation
Conversation
Integration test reportCommit: 584355c
Top 6 slowest tests (at least 2 minutes):
|
| } | ||
|
|
||
| func agentNameToAgentEnum(agentName string) protos.SshAgentShimAgent { | ||
| switch agentName { |
There was a problem hiding this comment.
[nice to have] The switch in agentNameToAgentEnum repeats the literals "claude" and "codex" from supportedAgents. The style guide asks for a named constant when a value is used in more than one place. If someone renames an agent in only one place, the event reports TYPE_UNSPECIFIED. Define package-level constants for the agent names and use them in both places, like agentType in cmd/aitools/telemetry.go. Another option: add a telemetryAgent protos.SshAgentShimAgent field to agentSpec, and use agentByName to find it.
| SshAgentShimErrorCategoryUnknown SshAgentShimErrorCategory = "UNKNOWN" | ||
| ) | ||
|
|
||
| // The agent for an `ssh agent-shim <agent>` command invocation. Values are in |
There was a problem hiding this comment.
[nit] Start the type comment with the type name, as SshAgentShimErrorCategory does. For example: "SshAgentShimAgent mirrors SshAgentShimAgentType.Type in the databricks_cli lumberjack proto." The comment says "JSON->proto serialization", but this step is deserialization. Remove the extra space in // The agent the user asked to launch. Add a doc comment to agentNameToAgentEnum that starts with the function name.
| } | ||
|
|
||
| func TestBuildAgentShimEvent(t *testing.T) { | ||
| for _, agent := range supportedAgents { |
There was a problem hiding this comment.
[nit] The loop only checks that each supported agent maps to a value other than SshAgentShimAgentUnspecified. No test covers the default branch. Add one case: an unknown name, such as "foo", maps to SshAgentShimAgentUnspecified.
Changes
Replace
agent_namewithagentand make the values match JSON->proto deserialization for enums (e.g. "CLAUDE_CODE" instead of "claude")Why
The original PR to add this event didn't follow telemetry best practices, this PR fixes that
Tests
automated tests pass