build!: convert to typescript #923
Conversation
googlebot
added
the
cla: yes
Codecov Report
@@ Coverage Diff @@
## master #923 +/- ##
=======================================
Coverage 94.06% 94.06%
=======================================
Files 23 23
Lines 10662 10662
Branches 503 503
=======================================
Hits 10029 10029
Misses 630 630
Partials 3 3 Continue to review full report at Codecov.
|
googlebot
added
cla: no
|
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
| @@ -50,7 +50,7 @@ function main( | |||
| }); | |||
|
|
|||
| async function publishWithRetrySettings() { | |||
| const formattedTopic = publisherClient.topicPath(projectId, topicName); | |||
| const formattedTopic = publisherClient.projectTopicPath(projectId, topicName); | |||
There was a problem hiding this comment.
I am worried this breaking change will get lost in the shuffle for folks. Can we make this super clear in the breaking changes section of the change log?
There was a problem hiding this comment.
Make sense. But not very clear here, should I update change log in this PR?
There was a problem hiding this comment.
@bcoe what's the recommended way to do this? I've done longer form "before / after" descriptions for breaking changes that were meaningful in the past. Is the process here to manually update the CHANGELOG in a followup PR?
| clients = ['publisher', 'subscriber'] | ||
| for client_name in clients: | ||
| client_file = f'src/v1/{client_name}_client.ts' | ||
| s.replace(client_file, |
There was a problem hiding this comment.
Oh hey, this is great!
| r"https://\1)") | ||
| # fix tslint issue due to mismatch gts version with gapic-generator-typescript | ||
| # it should be removed once pubsub upgrade gts 2.0.0 | ||
| s.replace(client_file, '\/\/ eslint\-disable\-next\-line\ \@typescript\-eslint\/no\-explicit\-any', |
There was a problem hiding this comment.
The TODO above says we need to upgrade to gts@2.0.0. That happens here right? Can we get rid of this synth change?
tsconfig.json
Outdated
| "outDir": "build", | ||
| "resolveJsonModule": true, | ||
| "lib": [ | ||
| "es2016", |
| @@ -22,7 +22,7 @@ import {promisifyAll} from '@google-cloud/promisify'; | |||
| import arrify = require('arrify'); | |||
| import {CallOptions} from 'google-gax'; | |||
|
|
|||
| import {google} from '../proto/iam'; | |||
| import {google} from '../protos/protos'; | |||
|
|
|||
There was a problem hiding this comment.
what does this iam.tsdo? some exported type like SetPolicyResponse can be found in protos.google.iam.v1.IAMPolicy. Thanks!
| @@ -50,7 +50,7 @@ function main( | |||
| }); | |||
|
|
|||
| async function publishWithRetrySettings() { | |||
| const formattedTopic = publisherClient.topicPath(projectId, topicName); | |||
| const formattedTopic = publisherClient.projectTopicPath(projectId, topicName); | |||
There was a problem hiding this comment.
@bcoe what's the recommended way to do this? I've done longer form "before / after" descriptions for breaking changes that were meaningful in the past. Is the process here to manually update the CHANGELOG in a followup PR?
| "prettier": "^1.18.2", | ||
| "proxyquire": "^2.0.0", | ||
| "sinon": "^9.0.0", | ||
| "source-map-support": "^0.5.9", | ||
| "ts-loader": "^6.2.1", | ||
| "typescript": "3.6.4", |
There was a problem hiding this comment.
Can we use "typescript": "^3.8.3" as we do for other libraries?
There was a problem hiding this comment.
As discussion with @feywind, upgrade typescript ^3.8.3 will happen when pubsub is ready for deprecate Node 8 and upgrade gts@2.0.0.
|
👏 👏 👏 |
BREAKING CHANGE：
topicPathhas been changed toprojectTopicPathSome synthtool hack will be removed once pubsub upgrade to
gts:2.0.0