-
Notifications
You must be signed in to change notification settings - Fork 518
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): migrate /doc and /dev to zipStream #6472
Conversation
/runIntegrationTests |
a2136c9
to
d0194dd
Compare
The duplicate code CI finding is on lines untouched by this PR (previously written unit tests) |
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.
🎉 🎉 🎉 nice
@@ -48,6 +48,7 @@ export * from './localizedText' | |||
export * as env from './vscode/env' | |||
export * from './vscode/commands2' | |||
export * from './utilities/pathUtils' | |||
export * from './utilities/zipStream' |
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 be exported as * as zipStream
@@ -150,4 +154,13 @@ export class ZipStream { | |||
streamBuffer: this._streamBuffer, | |||
} | |||
} | |||
|
|||
public static async unzip(zipBuffer: Buffer): Promise<ZipReaderResult[]> { |
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.
This isn't a streaming interface. So consumers of this function will be limited by available memory.
## Problem Users see a `Sorry, I ran into an issue while trying to upload your code` error when using /dev or /doc if files in the repo have a modified or created time before the MSDOS epoch. This affects some users who Remote SSH into a Linux machine. Logs show an out of range exception thrown from the adm-zip package. Adm-zip issue reported [here](cthackers/adm-zip#485) ## Solution Switch to the internal [zipStream module](https://github.com/aws/aws-toolkit-vscode/blob/master/packages/core/src/shared/utilities/zipStream.ts) --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). - License: I confirm that my contribution is made under the terms of the Apache 2.0 license.
Problem
Users see a
Sorry, I ran into an issue while trying to upload your code
error when using /dev or /doc if files in the repo have a modified or created time before the MSDOS epoch. This affects some users who Remote SSH into a Linux machine.Logs show an out of range exception thrown from the adm-zip package. Adm-zip issue reported here
Solution
Switch to the internal zipStream module
feature/x
branches will not be squash-merged at release time.