Skip to content
This repository has been archived by the owner on Jul 13, 2023. It is now read-only.

feat!: converts library to TypeScript adding v1p1beta1 surface #250

Merged
merged 7 commits into from
Jan 30, 2020

Conversation

bcoe
Copy link
Contributor

@bcoe bcoe commented Jan 25, 2020

converts library to TypeScript, so that we can generate v1p1beta1 which uses the new type annotations.

Currently there are the following generation issues that need to be addressed:

  • unit tests fail for all but `v1p1beta1.
  • v1beta1 API surface does not generate.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jan 25, 2020
@alexander-fenster alexander-fenster self-assigned this Jan 25, 2020
@alexander-fenster
Copy link
Contributor

I will take a look, thank you!

@alexander-fenster
Copy link
Contributor

@bcoe Unfortunately, asset API versions other than v1p1beta1 are still not ready for micro-generators (including TypeScript). Including @noahdietz here for details on this API conversion.

If you'd like to, we can combine TypeScript v1p1beta1 with JavaScript (generated by old gapic generator) into one library (manually, as we did for translate some time ago), but I'd rather not do it until we have an ETA about this API complete conversion.

@noahdietz
Copy link

This API version was added on Jan 06, 2020 and I didn't know, so I will need to annotate and publish. It will be done this week, I'll try to get it done sooner rather than later.

@noahdietz
Copy link

Asset v1p1beta1 was annotated and published here googleapis/googleapis@26ccb21.

@bcoe bcoe changed the title feat!: converts library to TypeScript adding v1p1beta1 surface feat: converts library to TypeScript adding v1p1beta1 surface Jan 30, 2020
package.json Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Jan 30, 2020

Codecov Report

Merging #250 into master will increase coverage by 55.53%.
The diff coverage is 90.45%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master     #250       +/-   ##
===========================================
+ Coverage   35.35%   90.89%   +55.53%     
===========================================
  Files          39        9       -30     
  Lines        6624     3392     -3232     
  Branches        0      151      +151     
===========================================
+ Hits         2342     3083      +741     
+ Misses       4282      305     -3977     
- Partials        0        4        +4
Impacted Files Coverage Δ
src/index.ts 100% <100%> (ø)
src/v1beta1/index.ts 100% <100%> (ø)
src/v1p2beta1/index.ts 100% <100%> (ø)
src/v1p1beta1/index.ts 100% <100%> (ø)
src/v1/index.ts 100% <100%> (ø)
src/v1beta1/asset_service_client.ts 90.44% <87.25%> (ø)
src/v1p2beta1/asset_service_client.ts 87.38% <87.38%> (ø)
src/v1/asset_service_client.ts 91.35% <91.35%> (ø)
src/v1p1beta1/asset_service_client.ts 92.62% <92.62%> (ø)
... and 4 more

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 18feb68...cf3b9f1. Read the comment docs.

src/index.ts Outdated Show resolved Hide resolved
// This API contains "path templates"; forward-slash-separated
// identifiers to uniquely identify resources within the API.
// Create useful helper objects for these.
this._pathTemplates = {
Copy link
Contributor

Choose a reason for hiding this comment

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

The list of path templates is different (if you compare it with the deleted src/v1/asset_service_client.js). Let me double check if the new path templates are correct in the annotations. For now, please mark this PR as feat! because it now has breaking changes.

@bcoe bcoe changed the title feat: converts library to TypeScript adding v1p1beta1 surface feat!: converts library to TypeScript adding v1p1beta1 surface Jan 30, 2020
synth.py Outdated Show resolved Hide resolved
output: {
library: 'asset',
filename: './asset.js',
library: 'AssetService',
Copy link
Contributor

Choose a reason for hiding this comment

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

This will be a breaking change to Webpack users. If we're breaking it anyway (which is what is likely to happen), let's keep this. If we want to be compatible, we'll need --main-service-name asset as an extra generator arg. But I'd better break it since there are no active Webpack users here as I'm pretty sure :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants