Create a centralized CommandAttributes and add attributes for DTS setup
#4649
Conversation
| import { type ActivityAttributes } from "@microsoft/vscode-azext-utils"; | ||
|
|
||
| export class CommandAttributes { | ||
|
|
There was a problem hiding this comment.
Nit: Consider localizing user-facing strings. The description and troubleshooting entries appear user-visible via activity UI; wrapping in localize() would keep consistency with the extension’s localization pattern.
Review generated with Copilot
There was a problem hiding this comment.
These are not user interfacing. maybe leave a comment in this file indicating that this is all for copilot?
|
|
||
| export class CommandAttributes { | ||
|
|
||
| static readonly Deploy: ActivityAttributes = { |
There was a problem hiding this comment.
Question: Would links help? ActivityAttributes might benefit from optional doc links per troubleshooting item (e.g., Learn docs for deployment and DTS). A links?: string[] or docLinks?: Record<string, string> could improve guidance.
Review generated with Copilot
| @@ -47,6 +48,8 @@ export async function getDTSConnectionIfNeeded(context: DTSConnectionContext, ap | |||
| const wizardContext: IDTSAzureConnectionWizardContext = { | |||
| ...context, | |||
| ...await createActivityContext({ withChildren: true }), | |||
| activityAttributes: CommandAttributes.DeployDTSSetup, | |||
There was a problem hiding this comment.
Telemetry correlation: callbackId: ${context.callbackId}.dtsSetup`` (line 51). Is this suffix convention documented/consumed elsewhere? If other components rely on exact formats, consider centralizing the pattern to avoid drift.
Review generated with Copilot
There was a problem hiding this comment.
We could also call with callWithTelemetryAndErrorHandling, but I thought that might be overkill when all we need to change is the callback id that shows up in the activity log. I wanted it to have its own callback id so it's easier for us to log in telemetry when users call into the activity log with copilot.
| @@ -68,25 +69,11 @@ async function deploy(actionContext: IActionContext, arg1: vscode.Uri | string | | |||
| } | |||
|
|
|||
| const context: IFuncDeployContext = Object.assign(actionContext, deployPaths, { | |||
There was a problem hiding this comment.
Import + duplication: You now call ...await createActivityContext() (line 71) and assign activityAttributes. Previously you spread a second createActivityContext() at the end. Verify there isn’t a second spread later (lines ~86–90) to prevent accidentally overwriting attributes.
Review generated with Copilot
There was a problem hiding this comment.
Don't think that this is a concern.
No description provided.