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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(types): augment types to manage response streams #2188

Merged
merged 1 commit into from May 27, 2020

Conversation

JustinBeckwith
Copy link
Contributor

@JustinBeckwith JustinBeckwith commented May 27, 2020

Fixes #2052, and adds a lot more TypeScript 馃槵

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label May 27, 2020
@codecov
Copy link

codecov bot commented May 27, 2020

Codecov Report

Merging #2188 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2188   +/-   ##
=======================================
  Coverage   84.31%   84.31%           
=======================================
  Files           9        9           
  Lines        1594     1594           
  Branches      129      110   -19     
=======================================
  Hits         1344     1344           
  Misses        250      250           

Continue to review full report at Codecov.

Legend - Click here to learn more
螖 = absolute <relative> (impact), 酶 = not affected, ? = missing data
Powered by Codecov. Last update 216135c...53e1788. Read the comment docs.

GlobalOptions,
BodyResponseCallback,
APIRequestContext,
} from 'googleapis-common';
import {Readable} from 'stream';
Copy link
Member

Choose a reason for hiding this comment

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

[suggestion, feel free to ignore]
should this be imported only if used by the given API? (otherwise, unused imports)

I must admit I have no idea if this applies to APIs in this library, but in GAPICs, we do put all such imports inside if statements in templates.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, in this particular case the import is only being used for the Readable type. It could be required for any of the API calls, if the user uses {responseType: 'stream'}

Copy link
Contributor

@sofisl sofisl left a 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 I totally understand the actual changes, but I understand the problem it's solving!

@JustinBeckwith JustinBeckwith merged commit 46ac8ba into master May 27, 2020
@JustinBeckwith JustinBeckwith deleted the so-readable branch December 13, 2020 18:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[TypeScript] The return type of drive.files.get is inferred wrongly when downloading as media
6 participants