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.

Sign up for GitHub

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

Jump to bottom

fix: TransferManager#downloadFileInChunks issues #2373

Merged
merged 7 commits into from Nov 29, 2023
Merged

fix: TransferManager#downloadFileInChunks issues #2373

merged 7 commits into from Nov 29, 2023

Conversation

rhodgkins
Copy link
Contributor

Fixes #2372 🦕

@product-auto-label product-auto-label bot added the size: s Pull request size is small. label Nov 28, 2023
@conventional-commit-lint-gcf Conventional Commit Lint GCF
Copy link

conventional-commit-lint-gcf bot commented Nov 28, 2023 •
edited

🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use automerge label. Good luck human!

-- conventional-commit-lint bot
https://conventionalcommits.org/

@product-auto-label product-auto-label bot added the api: storage Issues related to the googleapis/nodejs-storage API. label Nov 28, 2023
@rhodgkins rhodgkins mentioned this pull request Nov 28, 2023
Closed
@rhodgkins rhodgkins changed the title Fixes for #2372 fix: TransferManager#downloadFileInChunks issues #2372 Nov 28, 2023
@rhodgkins rhodgkins changed the title fix: TransferManager#downloadFileInChunks issues #2372 fix: TransferManager#downloadFileInChunks issues Nov 28, 2023
@rhodgkins rhodgkins marked this pull request as ready for review November 28, 2023 18:31
@rhodgkins rhodgkins requested review from a team as code owners November 28, 2023 18:31
@ddelgrosso1 ddelgrosso1 added the owlbot:run Add this label to trigger the Owlbot post processor. label Nov 28, 2023
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Nov 28, 2023
@ddelgrosso1 ddelgrosso1 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Nov 28, 2023
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Nov 28, 2023
ddelgrosso1
ddelgrosso1 approved these changes Nov 28, 2023
View reviewed changes
Copy link
Contributor

@ddelgrosso1 ddelgrosso1 left a comment

Choose a reason for hiding this comment

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

LGTM. We appreciate the contribution!

danielbankhead
danielbankhead reviewed Nov 28, 2023
View reviewed changes
Copy link
Member

@danielbankhead danielbankhead left a comment

Choose a reason for hiding this comment

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

LGTM - one nit, one suggestion

src/transfer-manager.ts Outdated Show resolved Hide resolved
src/transfer-manager.ts Show resolved Hide resolved
@rhodgkins @danielbankhead
Update src/transfer-manager.ts
2310d72 
Fixed typo

Co-authored-by: Daniel Bankhead <dan@danielbankhead.com>
@product-auto-label product-auto-label bot added size: m Pull request size is medium. and removed size: s Pull request size is small. labels Nov 29, 2023
@rhodgkins
Copy link
Contributor Author

Added some extra test cases too...

rhodgkins
rhodgkins commented Nov 29, 2023
View reviewed changes
src/transfer-manager.ts
Comment on lines -613 to +623
* @param {object} [file | string] {@link File} to download.
* @param {File | string} fileOrName {@link File} to download.
* @param {DownloadFileInChunksOptions} [options] Configuration options.
* @returns {Promise<DownloadResponse>}
* @returns {Promise<void | DownloadResponse>}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

...and fixed JSDoc declaration

@ddelgrosso1 ddelgrosso1 added the owlbot:run Add this label to trigger the Owlbot post processor. label Nov 29, 2023
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Nov 29, 2023
@ddelgrosso1 ddelgrosso1 added kokoro:force-run Add this label to force Kokoro to re-run the tests. owlbot:run Add this label to trigger the Owlbot post processor. labels Nov 29, 2023
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Nov 29, 2023
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Nov 29, 2023
@ddelgrosso1 ddelgrosso1 merged commit 65950f3 into googleapis:main Nov 29, 2023
16 checks passed
@release-please release-please bot mentioned this pull request Nov 29, 2023
Merged
@rhodgkins rhodgkins deleted the fix-2372 branch November 29, 2023 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danielbankhead danielbankhead danielbankhead approved these changes

@ddelgrosso1 ddelgrosso1 ddelgrosso1 approved these changes

Assignees
No one assigned
Labels
api: storage Issues related to the googleapis/nodejs-storage API. size: m Pull request size is medium.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Possible TransferManager#downloadFileInChunks issues
4 participants
@rhodgkins @danielbankhead @ddelgrosso1 @yoshi-kokoro