Skip to content

Conversation

@eamodio
Copy link
Member

@eamodio eamodio commented Feb 10, 2025

Adds async loaded product config
Refactors promo handling via product config
Moves all required content into promo model
Adds percentile support to promos

d13 and others added 2 commits February 7, 2025 16:45
Refactors promo handling via product config
Moves all required content into promo model
Adds percentile support to promos
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 25 out of 40 changed files in this pull request and generated no comments.

Files not reviewed (15)
  • contributions.json: Language not supported
  • package.json: Language not supported
  • src/plus/gk/utils/promo.utils.ts: Evaluated as low risk
  • src/constants.promos.ts: Evaluated as low risk
  • src/plus/gk/models/promo.ts: Evaluated as low risk
  • src/webviews/apps/home/components/ama-banner.ts: Evaluated as low risk
  • src/system/validation.ts: Evaluated as low risk
  • src/system/string.ts: Evaluated as low risk
  • src/container.ts: Evaluated as low risk
  • src/commands/git/worktree.ts: Evaluated as low risk
  • src/constants.context.ts: Evaluated as low risk
  • src/plus/gk/subscriptionService.ts: Evaluated as low risk
  • docs/telemetry-events.md: Evaluated as low risk
  • src/plus/launchpad/launchpad.ts: Evaluated as low risk
  • src/webviews/apps/home/components/integration-banner.ts: Evaluated as low risk
Comments suppressed due to low confidence (3)

src/constants.telemetry.ts:666

  • The 'json' field should always be a string to avoid potential issues when logging or processing this field. Consider initializing it as an empty string if no value is provided.
"json: string | undefined;"

src/commands/quickCommand.steps.ts:2658

  • [nitpick] The variable name 'promo' is ambiguous. It should be renamed to 'applicablePromo' for better readability.
const promo = await container.productConfig.getApplicablePromo(access.subscription.current.state, 'gate');

src/commands/quickCommand.steps.ts:2659

  • [nitpick] The error message string is unclear. It should provide more context about why the promo is required.
const detail = promo?.content?.quickpick.detail;

Copy link
Contributor

@axosoft-ramint axosoft-ramint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Once the percentile value is added into global telemetry, I think we're good to go.

@eamodio
Copy link
Member Author

eamodio commented Feb 10, 2025

Added in latest commit

@eamodio eamodio mentioned this pull request Feb 10, 2025
7 tasks
@eamodio eamodio merged commit fde18b2 into main Feb 10, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants