-
Notifications
You must be signed in to change notification settings - Fork 83
chore(amazonq): refactor codeWhispereServer #2373
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
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2373 +/- ##
==========================================
+ Coverage 60.63% 60.91% +0.28%
==========================================
Files 258 260 +2
Lines 58245 58313 +68
Branches 3532 3550 +18
==========================================
+ Hits 35316 35522 +206
+ Misses 22863 22724 -139
- Partials 66 67 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
221d336 to
25ba908
Compare
363554b to
026d993
Compare
|
|
||
| export class SessionResultsHandler { | ||
| constructor( | ||
| private readonly logging: any, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit, type should be Logging and Telementry?
| telemetryService, | ||
| credentialsProvider, | ||
| editsEnabled, | ||
| timeSinceLastUserModification, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we pass functions for editsEnabled and timeSinceLastUserModification like SessionResultsHandler below, otherwise handler won't pick up changes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes we should. I didn't catch it during testing. Thanks for pointing it out.
fbf0847 to
0b31636
Compare
|
/retryBuilds |
| private readonly getEditsEnabled: () => boolean, | ||
| private readonly getTimeSinceLastUserModification: () => number, | ||
| private readonly lsp: Lsp, | ||
| private readonly getTextDocument: ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will make a followup PR to move this method to a utils class later
Problem
codeWhispereServer.ts has more than 1000 lines which is hard to maintain and test individual components.
Solution
Refactor codeWhispereServer.ts into multiple components
Verify the inline and edit are working after refactoring:
Screen.Recording.2025-10-01.at.4.16.44.PM.mov
License
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.