Skip to content
This repository was archived by the owner on Mar 5, 2026. It is now read-only.

Migrate telemetry/recordEvent#2661

Merged
mkondratek merged 3 commits intomainfrom
mkondratek/chore/migrate-api-telemetry
Nov 19, 2024
Merged

Migrate telemetry/recordEvent#2661
mkondratek merged 3 commits intomainfrom
mkondratek/chore/migrate-api-telemetry

Conversation

@mkondratek
Copy link
Copy Markdown
Contributor

@mkondratek mkondratek commented Nov 18, 2024

This PR is a part of the protocol migration. Some endpoints are written by hand. We are switching to the protocol generated from Cody. In this PR:

Test plan

  1. All migrated endpoints have been verified with debugger (and reviewed in the trace log).

@mkondratek mkondratek self-assigned this Nov 18, 2024
Comment thread gradle.properties Outdated
nodeBinaries.commit=8755ae4c05fd476cd23f2972049111ba436c86d4
nodeBinaries.version=v20.12.2
cody.autocomplete.enableFormatting=true
cody.commit=6ac4a8c1831ad3945fc16f20b8f21947897d3d14
Copy link
Copy Markdown
Contributor Author

@mkondratek mkondratek Nov 18, 2024

Choose a reason for hiding this comment

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


public static final int spaceBetweenButtons = 5;

public static volatile String lastCopiedText = null;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It looks like since we have moved to webview we do not set this value. Hence, we do not log the paste event.

Or at any chance, does the agent handle the paste event? If it does not, we probably need to send the copied text via a notification to the client to track it in here.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@mkondratek mkondratek force-pushed the mkondratek/chore/migrate-api-telemetry branch 3 times, most recently from 4991dc9 to 2d4a79f Compare November 18, 2024 08:56
Copy link
Copy Markdown
Contributor

@pkukielka pkukielka left a comment

Choose a reason for hiding this comment

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

LGTM

mkondratek added a commit to sourcegraph/cody-public-snapshot that referenced this pull request Nov 18, 2024
This change is required for the ongoing migration to the generated API
in the JetBrains client.

Related PR: sourcegraph/jetbrains#2661

Some more advanced types are skipped to simplify the code gen. We used
internal types too casually. We should be more precise about the
protocol.

Related thread:
https://sourcegraph.slack.com/archives/C05AGQYD528/p1728049089597009

## Test plan

<!-- Required. See
https://docs-legacy.sourcegraph.com/dev/background-information/testing_principles.
-->

Verified with JB. Telemetry goes through the API.

## Changelog

<!-- OPTIONAL; info at
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c
-->
@mkondratek mkondratek force-pushed the mkondratek/chore/migrate-api-telemetry branch from f4168ad to 28368da Compare November 18, 2024 09:56
@mkondratek mkondratek force-pushed the mkondratek/chore/migrate-api-telemetry branch from 28368da to ca46fca Compare November 19, 2024 11:06
mkondratek added a commit that referenced this pull request Nov 19, 2024
@mkondratek mkondratek force-pushed the mkondratek/chore/migrate-api-telemetry branch from ca46fca to 485934a Compare November 19, 2024 11:28
@mkondratek mkondratek merged commit 6cf4688 into main Nov 19, 2024
@mkondratek mkondratek deleted the mkondratek/chore/migrate-api-telemetry branch November 19, 2024 11:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants