Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DEV2-4433 chat api usage #1397

Merged
merged 6 commits into from
Dec 5, 2023
Merged

DEV2-4433 chat api usage #1397

merged 6 commits into from
Dec 5, 2023

Conversation

dimacodota
Copy link
Contributor

@dimacodota dimacodota commented Dec 5, 2023

DEV2-4433 chat api usage

highlights:
the chat api is now a class that exposes

  • the handle Message api
  • handles the chat incoming requests

this class is inited differently from the saas extension and the enterprise extension

@dimacodota dimacodota requested a review from a team as a code owner December 5, 2023 12:12
isTelemetryEnabled: boolean;
};

export class ChatApi {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is now a class that is inited here
and here

].includes(vscode.window.activeColorTheme.kind),
isTelemetryEnabled: isCapabilityEnabled(Capability.ALPHA_CAPABILITY),
serverUrl,
isSelfHosted: context.workspaceState.get(
Copy link
Contributor Author

@dimacodota dimacodota Dec 5, 2023

Choose a reason for hiding this comment

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

@yonip23 this was changed to be injected from the ctor, and not managed as a flag

Copy link
Contributor Author

Choose a reason for hiding this comment

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

meant @yonip23 and not @yonidavidson 🤦‍♂️

@@ -111,15 +102,8 @@ export default class ChatViewProvider implements WebviewViewProvider {
});
}

waitForChatInitiated(): Promise<void> {
return new Promise((resolve) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@yairco1990 - this was changed to be promise based here

Copy link
Contributor

@yonip23 yonip23 left a comment

Choose a reason for hiding this comment

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

lgtm, nice change 👍🏻

Copy link
Contributor

@yairco1990 yairco1990 left a comment

Choose a reason for hiding this comment

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

lgtm

@dimacodota dimacodota merged commit 6ad2176 into master Dec 5, 2023
3 of 4 checks passed
@dimacodota dimacodota deleted the DEV2-4433 branch December 5, 2023 14:35
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