feat!: convert library to TypeScript, adding v1p1beta1 import #198

bcoe · 2020-01-25T01:31:20Z

This moves us to the TypeScript generator, which allows us to start pulling in fancy 👖 new updates to the API.

codecov · 2020-01-25T01:50:24Z

Codecov Report

Merging #198 into master will increase coverage by 64.37%.
The diff coverage is 100%.

@@             Coverage Diff             @@
##           master     #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

Impacted Files	Coverage Δ
src/v1/security_center_client.ts	`93.07% <ø> (ø)`
src/v1p1beta1/security_center_client.ts	`93.07% <ø> (ø)`
src/v1beta1/security_center_client.ts	`92.37% <ø> (ø)`
src/index.ts	`100% <100%> (ø)`
src/v1beta1/index.ts	`100% <100%> (ø)`
src/v1p1beta1/index.ts	`100% <100%> (ø)`
src/v1/index.ts	`100% <100%> (ø)`
... 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 3c536e6...b00e76c. Read the comment docs.

xiaozhenliu-gg5

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.

package.json

xiaozhenliu-gg5

LGTM with a minor question, thanks!

xiaozhenliu-gg5 · 2020-01-31T19:39:31Z

webpack.config.js

  output: {
-    library: 'security-center',
+    library: 'SecurityCenter',


Not sure if it's a breaking change for library name @alexander-fenster ?

alexander-fenster · 2020-01-31T20:40:48Z

This will be breaking because the set of path templates is different (we are missing some helper methods such as assetSecurityMarksPath(organization, asset)). This is not a problem with your generation, it's an annotation problem that I'm trying to figure out with my team.

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

LGTM with breaking changes - if it's not OK then let's wait for a while to get it sorted out

alexander-fenster · 2020-01-31T21:19:14Z

synth.py

-# [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/,


This is cool! I'm making this change internally, then we'll be able to remove this hack. Internal CL 292600161

alexander-fenster

Please wait for the generator update to generate all the path templates (even though they are not directly referenced).

alexander-fenster · 2020-01-31T21:35:13Z

googleapis/gapic-generator-typescript#220 will take care of this.

JustinBeckwith · 2020-01-31T22:28:49Z

package.json

@@ -1,18 +1,31 @@
 {
  "name": "@google-cloud/security-center",
  "description": "Cloud Security Command Center API client for Node.js",
-  "version": "2.3.2",
+  "version": "2.3.1",


Huh, the version drop is weird

xiaozhenliu-gg5 · 2020-02-04T20:00:00Z

Newest image is published and I generated security-center which picks the missing resources back, can you try one more time? @bcoe Thanks!

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jan 25, 2020

bcoe requested a review from xiaozhenliu-gg5 January 25, 2020 01:31

bcoe assigned alexander-fenster Jan 25, 2020

bcoe requested a review from alexander-fenster January 25, 2020 01:34

xiaozhenliu-gg5 reviewed Jan 27, 2020

View reviewed changes

package.json Outdated Show resolved Hide resolved

bcoe force-pushed the typescript-conversion branch from ed7dffb to 19688aa Compare January 31, 2020 18:58

xiaozhenliu-gg5 approved these changes Jan 31, 2020

View reviewed changes

alexander-fenster changed the title ~~feat: convert library to TypeScript, adding v1p1beta1 import~~ feat!: convert library to TypeScript, adding v1p1beta1 import Jan 31, 2020

alexander-fenster approved these changes Jan 31, 2020

View reviewed changes

alexander-fenster added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jan 31, 2020

kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jan 31, 2020

alexander-fenster reviewed Jan 31, 2020

View reviewed changes

alexander-fenster suggested changes Jan 31, 2020

View reviewed changes

JustinBeckwith approved these changes Jan 31, 2020

View reviewed changes

This was referenced Feb 1, 2020

feat!: generate all api resources and resource definition which is referenced. googleapis/gapic-generator-typescript#222

Merged

feat!: resource generation logic fix googleapis/gapic-generator-typescript#225

Merged

bcoe added 10 commits February 5, 2020 09:53

feat: convert library to TypeScript, pulling in v1p1beta1

2fbbf83

chore: fix docs test

c5dc88d

chore: fix linting

6629726

chore: re-run generator

8c02588

chore: re-run generator post merge

3773cee

build: fix environment

c3d873b

chore: address code review

5273e71

chore: header lint

06df202

chore: fix generation bug

f30f0cc

chore: run the generator

befe0c6

bcoe force-pushed the typescript-conversion branch from 71a44aa to befe0c6 Compare February 5, 2020 17:54

chore: cleanup package version

b00e76c

alexander-fenster approved these changes Feb 5, 2020

View reviewed changes

bcoe merged commit 0adb7d0 into master Feb 5, 2020

bcoe deleted the typescript-conversion branch February 5, 2020 18:53

release-please bot mentioned this pull request Feb 5, 2020

chore: release 3.0.0 #204

Merged

release-please bot mentioned this pull request May 18, 2022

chore(main): release 6.0.0 #488

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat!: convert library to TypeScript, adding v1p1beta1 import #198

feat!: convert library to TypeScript, adding v1p1beta1 import #198

bcoe commented Jan 25, 2020

codecov bot commented Jan 25, 2020 •

edited

Loading

xiaozhenliu-gg5 left a comment

xiaozhenliu-gg5 left a comment

xiaozhenliu-gg5 Jan 31, 2020

alexander-fenster commented Jan 31, 2020

alexander-fenster left a comment

alexander-fenster Jan 31, 2020

alexander-fenster left a comment

alexander-fenster commented Jan 31, 2020

JustinBeckwith Jan 31, 2020

xiaozhenliu-gg5 commented Feb 4, 2020

feat!: convert library to TypeScript, adding v1p1beta1 import #198

feat!: convert library to TypeScript, adding v1p1beta1 import #198

Conversation

bcoe commented Jan 25, 2020

codecov bot commented Jan 25, 2020 • edited Loading

Codecov Report

xiaozhenliu-gg5 left a comment

Choose a reason for hiding this comment

xiaozhenliu-gg5 left a comment

Choose a reason for hiding this comment

xiaozhenliu-gg5 Jan 31, 2020

Choose a reason for hiding this comment

alexander-fenster commented Jan 31, 2020

alexander-fenster left a comment

Choose a reason for hiding this comment

alexander-fenster Jan 31, 2020

Choose a reason for hiding this comment

alexander-fenster left a comment

Choose a reason for hiding this comment

alexander-fenster commented Jan 31, 2020

JustinBeckwith Jan 31, 2020

Choose a reason for hiding this comment

xiaozhenliu-gg5 commented Feb 4, 2020

codecov bot commented Jan 25, 2020 •

edited

Loading