feat!: convert library to TypeScript, adding v1p1beta1 import#198
feat!: convert library to TypeScript, adding v1p1beta1 import#198
Conversation
googlebot
added
the
cla: yes
Codecov Report
@@ Coverage Diff @@
## master googleapis/gapic-generator-typescript#198 +/- ##
===========================================
+ Coverage 28.56% 92.93% +64.37%
===========================================
Files 40 7 -33
Lines 11220 9965 -1255
Branches 0 266 +266
===========================================
+ Hits 3205 9261 +6056
+ Misses 8015 701 -7314
- Partials 0 3 +3
Continue to review full report at Codecov.
|
xiaozhenliu-gg5
left a comment
xiaozhenliu-gg5
left a comment
There was a problem hiding this comment.
Thanks for doing this Ben!
Seems the proto annotation for v1beta1 and v1p1beta1 is not published, so the path templates of resources are not generated properly in client.ts, which breaks the samples-test.
bcoe
force-pushed
the
typescript-conversion
branch
from
ed7dffb to
19688aa
Compare
xiaozhenliu-gg5
left a comment
xiaozhenliu-gg5
left a comment
There was a problem hiding this comment.
LGTM with a minor question, thanks!
| entry: './src/index.ts', | ||
| output: { | ||
| library: 'security-center', | ||
| library: 'SecurityCenter', |
There was a problem hiding this comment.
Not sure if it's a breaking change for library name @alexander-fenster ?
|
This will be breaking because the set of path templates is different (we are missing some helper methods such as We can either merge it as a breaking change now (and if annotations are fixed later, add them), or wait for a while until this is sorted out. I will let you know! |
alexander-fenster
changed the title
alexander-fenster
left a comment
alexander-fenster
left a comment
There was a problem hiding this comment.
LGTM with breaking changes - if it's not OK then let's wait for a while to get it sorted out
alexander-fenster
added
the
kokoro:force-run
kokoro-team
removed
the
kokoro:force-run
| # [END fix-dead-link] | ||
| # There's an internal tracking ticket for this generation bug b/148673437 | ||
| # we should remove this replacement hack as soon as it is addressed: | ||
| s.replace("protos/google/cloud/securitycenter/v1p1beta1/notification_config.proto", r"""// The Pub/Sub Topic resource definition is in google/cloud/pubsub/v1/, |
There was a problem hiding this comment.
This is cool! I'm making this change internally, then we'll be able to remove this hack. Internal CL 292600161
alexander-fenster
left a comment
alexander-fenster
left a comment
There was a problem hiding this comment.
Please wait for the generator update to generate all the path templates (even though they are not directly referenced).
|
googleapis/google-cloud-node-core#673 will take care of this. |
| "name": "@google-cloud/security-center", | ||
| "description": "Cloud Security Command Center API client for Node.js", | ||
| "version": "2.3.2", | ||
| "version": "2.3.1", |
There was a problem hiding this comment.
Huh, the version drop is weird
|
Newest image is published and I generated |
bcoe
force-pushed
the
typescript-conversion
branch
from
71a44aa to
befe0c6
Compare
6 participants
This moves us to the TypeScript generator, which allows us to start pulling in fancy 👖 new updates to the API.