Skip to content

Conversation

@ivikash
Copy link
Member

@ivikash ivikash commented Nov 5, 2024

Create an abstract class that can be used by all connectors.


License: I confirm that my contribution is made under the terms of the Apache 2.0 license.

@ivikash ivikash requested a review from a team as a code owner November 5, 2024 21:51
@github-actions
Copy link

github-actions bot commented Nov 5, 2024

  • This pull request modifies code in src/* but no tests were added/updated.
    • Confirm whether tests should be added or ensure the PR description explains why tests are not required.

/**
* We've pressed on a followup button and should start watching that round trip telemetry
*/
this.sendMessageToExtension({
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd probably move these out of here for now and into the cwchatconnector. I haven't checked whether or not the entire chat round trip telemetry flow works for gumby/feature dev yet

Copy link
Member Author

Choose a reason for hiding this comment

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

Discussed Offline - amazonq_chatRoundTrip is not getting emitted for /dev, so this is good to go.

Copy link
Contributor

Choose a reason for hiding this comment

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

file name should be camelcase (baseConnector.ts)

Copy link
Contributor

@justinmk3 justinmk3 left a comment

Choose a reason for hiding this comment

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

Nice! Are there any tests that could be de-duplicated too?

@ivikash ivikash force-pushed the refactor/base-connector branch from b364a52 to 7c53a67 Compare November 5, 2024 22:21
@ivikash ivikash force-pushed the refactor/base-connector branch from 7c53a67 to 5677737 Compare November 5, 2024 22:26
@ivikash ivikash merged commit 89375c4 into aws:master Nov 6, 2024
23 of 25 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