-
Notifications
You must be signed in to change notification settings - Fork 437
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
feat(amazonq): Add command to generate unit tests for selected code #5577
Conversation
…scode into add-generate-tests
This pull request modifies files 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. |
This pull request modifies a feature or fixes a bug, but it does not include a changelog entry. All pull requests that introduce new features or bug fixes must have a corresponding changelog item describing the changes. |
packages/core/src/codewhispererChat/controllers/chat/userIntent/userIntentRecognizer.ts
Outdated
Show resolved
Hide resolved
packages/core/src/codewhispererChat/commands/registerCommands.ts
Outdated
Show resolved
Hide resolved
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. |
This pull request implements a feature or fix, so it must include a changelog entry. See CONTRIBUTING.md#changelog for instructions. |
@@ -223,6 +245,8 @@ export class Auth implements AuthService, ConnectionManager { | |||
this.#onDidChangeActiveConnection.fire(conn) | |||
await this.store.setCurrentProfileId(id) | |||
|
|||
await setContext('aws.isInternalUser', this.isInternalAmazonUser()) |
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.
I was wondering if forgetConnection
and expireConnection
should also un-set this. But it's probably useful to keep this hint around, once it was deduced. And in that case, should this flag be stored in globalState? Then we don't need to wait until a valid connection next time.
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.
I don't think it hurts to set this back to false on forgetConnection
and expireConnection
.
Some testing that I did made me doubt how globalState
worked. Using globalState.update
for this didn't work to enable the feature for internal users. setContext
, however, worked. Are global keys detectable from package.json after being set?
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.
Are global keys detectable from package.json after being set?
No
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.
Oh okay well that would explain it. We need this to be set with setContext
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.
LGTM after comments. Needs approval from someone on your team too.
Co-authored-by: Justin M. Keyes <[email protected]>
- Add command to Amazon Q to generate unit tests for selected code. The feature will be limited to internal Amazon users, initially. - Add "Generate Tests" command.
- Add command to Amazon Q to generate unit tests for selected code. The feature will be limited to internal Amazon users, initially. - Add "Generate Tests" command.
Adds command to Amazon Q to generate unit tests for selected code. The feature will be limited to internal Amazon users to collect usage metrics and, subsequently, will be released to all users.
Types of changes
Description
Checklist
License
I confirm that my contribution is made under the terms of the Apache 2.0 license.