feat!: monitoring convert to typescript #360

summer-ji-eng · 2020-02-11T08:24:16Z

BREAKING CHANGE: generated client in typescript changed the path template helper names

deleted js file and add js from generator
update package.json
correct the path of template in the samples test
npm install 👍
npm test👍
npm run lint👍
npm run docs-test👍
npm run system-test👍
npm run samples-test👍

codecov · 2020-02-11T08:31:58Z

Codecov Report

❗ No coverage uploaded for pull request base (master@ebcc56c). Click here to learn what that means.
The diff coverage is 75.3%.

@@            Coverage Diff            @@
##             master     #360   +/-   ##
=========================================
  Coverage          ?   76.59%           
=========================================
  Files             ?        8           
  Lines             ?    14628           
  Branches          ?      287           
=========================================
  Hits              ?    11205           
  Misses            ?     3417           
  Partials          ?        6

Impacted Files	Coverage Δ
src/v3/notification_channel_service_client.ts	`78.89% <ø> (ø)`
src/v3/metric_service_client.ts	`76.04% <ø> (ø)`
src/v3/uptime_check_service_client.ts	`75.39% <ø> (ø)`
src/v3/service_monitoring_service_client.ts	`78.05% <ø> (ø)`
src/index.ts	`100% <100%> (ø)`
src/v3/index.ts	`100% <100%> (ø)`
src/v3/alert_policy_service_client.ts	`74.28% <74.28%> (ø)`
src/v3/group_service_client.ts	`75.5% <75.5%> (ø)`

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 ebcc56c...da559f4. Read the comment docs.

package.json

smoke-test/metric_service_smoke_test.ts

synth.py

alexander-fenster · 2020-02-11T15:57:55Z

Good news is that system tests pass on Kokoro (that means that the permission denied error is something related to your account setup).

As for samples tests, there are some path templates missing which make them fail. We need to discuss with @xiaozhenliu-gg5 (who knows most about path templates :) ) and see if their omission is indended or not.

xiaozhenliu-gg5 · 2020-02-11T18:24:21Z

As for samples tests, there are some path templates missing which make them fail. We need to discuss with @xiaozhenliu-gg5 (who knows most about path templates :) ) and see if their omission is indended or not.

this should be fixed by googleapis/gapic-generator-typescript#239

alexander-fenster · 2020-02-12T00:53:10Z

@summer-ji-eng since it has breaking changes now (path template helper names), I changed the title to feat!: ...., it will force our bot to release it as a major version.

summer-ji-eng · 2020-02-12T00:59:21Z

@summer-ji-eng since it has breaking changes now (path template helper names), I changed the title to feat!: ...., it will force our bot to release it as a major version.

Agree. Thanks for reminding.

package.json

alexander-fenster

LGTM with one nit (google-gax version). Thank you!

bcoe

we should either try to get onto @JustinBeckwith's@2.x.x branch of gts, or add eslint as well for the JavaScript files in the samples directory 👌

otherwise things are looking good to me.

JustinBeckwith

Mostly LGTM. I'd like to block on making sure we fix URLs in the upstream monitoring protos and get them synced to third party. If it's broken here, it's broken 7 other times.

package.json

synth.py

summer-ji-eng · 2020-02-12T23:07:08Z

Mostly LGTM. I'd like to block on making sure we fix URLs in the upstream monitoring protos and get them synced to third party. If it's broken here, it's broken 7 other times.

Thanks Justin, could you show me how to fix URLs or who should I contact to fix URLs?

UPDATE:: Alex showed me how. I will fix the URLs in upstream.

alexander-fenster · 2020-02-12T23:19:56Z

@summer-ji-eng Can you try adding back the prelint script to make GitHub lint action pass? This is the new CI, the manual and old PRs don't have anything like this.

As for the source protos, I'll show where they are in the chat room.

summer-ji-eng · 2020-02-13T03:22:04Z

we should either try to get onto @JustinBeckwith's@2.x.x branch of gts, or add eslint as well for the JavaScript files in the samples directory 👌

otherwise things are looking good to me.

@JustinBeckwith Could you give us suggestion here? Is @2.x.x gtscan catch up linting issues with Javascript, if not, should I add eslint on the top level or just samples directory? Thanks

JustinBeckwith · 2020-02-13T03:51:24Z

I don't think we want to start messing with gts@2 until we drop support for node 8. Even then, I would prefer we roll it out all at once. Instead, I think we need to:

Make sure to add the prelint npm script back
Change the fix npm script to "fix": "gts fix && eslint 'samples/**/*.js' --fix",
Locally, cd samples && npm install && cd .. && npm run fix
Push the changes

I think we were missing the change in the fix script that caused the linter to mess up :)

…toring into summer_conversion

JustinBeckwith · 2020-02-13T15:27:43Z

package.json

@@ -32,9 +32,16 @@
    "samples-test": "cd samples/ && npm link ../ && npm install && npm test && cd ../",
    "system-test": "mocha build/system-test",
    "test": "c8 mocha build/test",
+<<<<<<< HEAD


that don't look right :)

Weird, it doesn't show on my local. The latest commit doesn't contain these "HEAD".

bcoe

this is looking good to me; we should make sure before we merge that we clearly document what changes are breaking (that we know of).

change the body of your commit message when squashing to:

BREAKING CHANGE: description of breaking changes.

👍

package.json

alexander-fenster · 2020-02-13T19:44:23Z

@summer-ji-eng Please regen the code once more (run synthtool) because one more change just came in: googleapis/googleapis@56b55aa

It might change the code and we want to include it here.

JustinBeckwith

Perfect! Well done 🎉

summer-ji-eng requested review from JustinBeckwith, alexander-fenster, yoshi-automation and xiaozhenliu-gg5 February 11, 2020 08:24

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

feat: monitoring conversion by delete js file and add ts from generator

2a86bab

summer-ji-eng force-pushed the summer_conversion branch from 6336365 to 2a86bab Compare February 11, 2020 08:31

JustinBeckwith suggested changes Feb 11, 2020

View reviewed changes

package.json Outdated Show resolved Hide resolved

package.json Outdated Show resolved Hide resolved

smoke-test/metric_service_smoke_test.ts Outdated Show resolved Hide resolved

synth.py Outdated Show resolved Hide resolved

summer-ji-eng added 4 commits February 11, 2020 15:14

address the comments

059d06e

fixed samples test by modify the matched path

8a0b5e7

fix samples test for notificationChannelPath

25fe8ee

fix samples test alertPolicyPath

644e856

summer-ji-eng requested a review from JustinBeckwith February 12, 2020 00:21

fix lint

1b38080

alexander-fenster changed the title ~~feat: monitoring convert to typescript~~ feat!: monitoring convert to typescript Feb 12, 2020

xiaozhenliu-gg5 approved these changes Feb 12, 2020

View reviewed changes

alexander-fenster reviewed Feb 12, 2020

View reviewed changes

package.json Outdated Show resolved Hide resolved

alexander-fenster approved these changes Feb 12, 2020

View reviewed changes

summer-ji-eng and others added 2 commits February 11, 2020 23:11

upgrade google-gax

ac73c44

Merge branch 'master' into summer_conversion

8047a58

bcoe suggested changes Feb 12, 2020

View reviewed changes

JustinBeckwith suggested changes Feb 12, 2020

View reviewed changes

package.json Show resolved Hide resolved

package.json Show resolved Hide resolved

package.json Show resolved Hide resolved

synth.py Outdated Show resolved Hide resolved

summer-ji-eng added 10 commits February 12, 2020 23:44

feat: monitoring conversion by delete js file and add ts from generator

eed1cc7

address the comments

5ea7874

fixed samples test by modify the matched path

8ee50b6

fix samples test for notificationChannelPath

1e7ca72

fix samples test alertPolicyPath

637af44

fix lint

af6efb9

upgrade google-gax

ebcc56c

prelint, add eslint for samples test

82689c6

Merge branch 'summer_conversion' of github.com:googleapis/nodejs-moni…

f24e44a

…toring into summer_conversion

merge head

99bda5f

JustinBeckwith reviewed Feb 13, 2020

View reviewed changes

bcoe approved these changes Feb 13, 2020

View reviewed changes

package.json Show resolved Hide resolved

fix upstream proto links, get rid of synth hack

da559f4

JustinBeckwith approved these changes Feb 14, 2020

View reviewed changes

summer-ji-eng merged commit 5bbd963 into master Feb 18, 2020

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

chore: release 2.0.0 #364

Closed

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

chore: release 2.0.0 #377

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat!: monitoring convert to typescript #360

feat!: monitoring convert to typescript #360

summer-ji-eng commented Feb 11, 2020 •

edited

Loading

codecov bot commented Feb 11, 2020 •

edited

Loading

alexander-fenster commented Feb 11, 2020

xiaozhenliu-gg5 commented Feb 11, 2020 •

edited

Loading

alexander-fenster commented Feb 12, 2020

summer-ji-eng commented Feb 12, 2020

alexander-fenster left a comment

bcoe left a comment

JustinBeckwith left a comment

summer-ji-eng commented Feb 12, 2020 •

edited

Loading

alexander-fenster commented Feb 12, 2020

summer-ji-eng commented Feb 13, 2020

JustinBeckwith commented Feb 13, 2020

JustinBeckwith Feb 13, 2020

summer-ji-eng Feb 14, 2020

bcoe left a comment

alexander-fenster commented Feb 13, 2020

JustinBeckwith left a comment

feat!: monitoring convert to typescript #360

feat!: monitoring convert to typescript #360

Conversation

summer-ji-eng commented Feb 11, 2020 • edited Loading

codecov bot commented Feb 11, 2020 • edited Loading

Codecov Report

alexander-fenster commented Feb 11, 2020

xiaozhenliu-gg5 commented Feb 11, 2020 • edited Loading

alexander-fenster commented Feb 12, 2020

summer-ji-eng commented Feb 12, 2020

alexander-fenster left a comment

Choose a reason for hiding this comment

bcoe left a comment

Choose a reason for hiding this comment

JustinBeckwith left a comment

Choose a reason for hiding this comment

summer-ji-eng commented Feb 12, 2020 • edited Loading

alexander-fenster commented Feb 12, 2020

summer-ji-eng commented Feb 13, 2020

JustinBeckwith commented Feb 13, 2020

JustinBeckwith Feb 13, 2020

Choose a reason for hiding this comment

summer-ji-eng Feb 14, 2020

Choose a reason for hiding this comment

bcoe left a comment

Choose a reason for hiding this comment

alexander-fenster commented Feb 13, 2020

JustinBeckwith left a comment

Choose a reason for hiding this comment

summer-ji-eng commented Feb 11, 2020 •

edited

Loading

codecov bot commented Feb 11, 2020 •

edited

Loading

xiaozhenliu-gg5 commented Feb 11, 2020 •

edited

Loading

summer-ji-eng commented Feb 12, 2020 •

edited

Loading