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

feat(amazonq): auto-download results #6519

Merged
merged 2 commits into from
Feb 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"type": "Feature",
"description": "/transform: automatically download results when ready"
}
Original file line number Diff line number Diff line change
Expand Up @@ -773,6 +773,12 @@ export async function postTransformationJob() {
if (transformByQState.getPayloadFilePath() !== '') {
fs.rmSync(transformByQState.getPayloadFilePath(), { recursive: true, force: true }) // delete ZIP if it exists
}

// attempt download for user
// TODO: refactor as explained here https://github.com/aws/aws-toolkit-vscode/pull/6519/files#r1946873107
if (transformByQState.isSucceeded() || transformByQState.isPartiallySucceeded()) {
await vscode.commands.executeCommand('aws.amazonq.transformationHub.reviewChanges.startReview')
Copy link
Contributor

Choose a reason for hiding this comment

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

Just as a random aside, ideally this would have used our internal Commands wrapper core/src/shared/vscode/commands2.ts. This wrapper basically allows you to define the command as a const that you can execute later which is much more robust then having to pass around command ids!

In order to do that you would need to register aws.amazonq.transformationHub.reviewChanges.startReview like:

export const startReview = Commands.declare('aws.amazonq.transformationHub.reviewChanges.startReview',
    () => async () => {
       // implement me here
    }
)

and then here you could just do startReview.tryExecute() that way you don't have to care about underlying id's!

See: https://github.com/aws/aws-toolkit-vscode/blob/master/docs/arch_develop.md#commands for more info

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Noted, https://github.com/aws/aws-toolkit-vscode/blob/master/packages/core/src/codewhisperer/service/transformByQ/transformationResultsViewProvider.ts is full of registered commands that could be refactored this way, so I think it's better if I do all of that in a separate PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm okay with merging this as is as long as you commit/agree to making that change 😄. It should be like 15 minutes worth of work tops

Copy link
Contributor Author

@dhasani23 dhasani23 Feb 7, 2025

Choose a reason for hiding this comment

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

Added a TODO comment and made a note of it so I don't forget

}
}

export async function transformationJobErrorHandler(error: any) {
Expand Down
Loading