build!: convert to typescript by summer-ji-eng · Pull Request #923 · googleapis/nodejs-pubsub

summer-ji-eng · 2020-03-23T08:22:35Z

BREAKING CHANGE： topicPath has been changed to projectTopicPath

Some synthtool hack will be removed once pubsub upgrade to gts:2.0.0

…into summer_convert

codecov · 2020-03-24T00:19:37Z

Codecov Report

Merging #923 into master will not change coverage by %.
The diff coverage is n/a.

@@           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.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4acc3e9...4acc3e9. Read the comment docs.

googlebot · 2020-04-07T22:17:45Z

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

JustinBeckwith · 2020-04-08T16:02:43Z


  async function publishWithRetrySettings() {
-    const formattedTopic = publisherClient.topicPath(projectId, topicName);
+    const formattedTopic = publisherClient.projectTopicPath(projectId, topicName);


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?

Make sense. But not very clear here, should I update change log in this PR?

@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?

JustinBeckwith · 2020-04-08T16:04:42Z

+clients = ['publisher', 'subscriber']
+for client_name in clients:
+    client_file = f'src/v1/{client_name}_client.ts'
+    s.replace(client_file,


Oh hey, this is great!

JustinBeckwith · 2020-04-08T16:05:21Z

-        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',


The TODO above says we need to upgrade to [email protected]. That happens here right? Can we get rid of this synth change?

JustinBeckwith · 2020-04-08T16:06:05Z

+    "outDir": "build",
+    "resolveJsonModule": true,
+    "lib": [
+      "es2016",


Still says 2016 :)

xiaozhenliu-gg5 · 2020-04-08T16:08:51Z


-import {google} from '../proto/iam';
+import {google} from '../protos/protos';



what does this iam.tsdo? some exported type like SetPolicyResponse can be found in protos.google.iam.v1.IAMPolicy. Thanks!

xiaozhenliu-gg5

Overall LGTM! Thank you Summer!

JustinBeckwith · 2020-04-08T19:04:50Z


  async function publishWithRetrySettings() {
-    const formattedTopic = publisherClient.topicPath(projectId, topicName);
+    const formattedTopic = publisherClient.projectTopicPath(projectId, topicName);


@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?

feywind

On re-review, this looks pretty nice! I would take another look down the remaining comments, but even as it is, it's fine. We can always tweak it a bit before 2.0.0 if something comes up.

alexander-fenster · 2020-04-09T17:59:58Z

    "sinon": "^9.0.0",
    "source-map-support": "^0.5.9",
+    "ts-loader": "^6.2.1",
    "typescript": "3.6.4",


Can we use "typescript": "^3.8.3" as we do for other libraries?

As discussion with @feywind, upgrade typescript ^3.8.3 will happen when pubsub is ready for deprecate Node 8 and upgrade [email protected].

alexander-fenster

LGTM!

JustinBeckwith · 2020-04-09T18:48:02Z

👏 👏 👏

* docs: fix families typo * docs: fix initial typo

summer-ji-eng added 2 commits March 23, 2020 00:54

feat: convert client to typescript

7276eca

upgrade package.json

2af5c35

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

summer-ji-eng added 6 commits March 23, 2020 01:29

revert changes on ci.yaml

7080c54

feat: convert client to typescript

e8206af

upgrade package.json

2f32e34

revert changes on ci.yaml

c848fe7

update synthtool

93d35f1

Merge branch 'summer_convert' of github.com:googleapis/nodejs-pubsub …

7e2f1dc

…into summer_convert

summer-ji-eng requested a review from alexander-fenster March 24, 2020 00:41

summer-ji-eng assigned feywind and unassigned feywind Mar 24, 2020

summer-ji-eng requested a review from feywind March 24, 2020 00:41

summer-ji-eng added 16 commits March 23, 2020 20:29

feat: convert client to typescript

a4e8f90

upgrade package.json

9d7e28a

revert changes on ci.yaml

ffa02bf

update synthtool

f42a1e0

feat: convert client to typescript

153e188

revert changes on ci.yaml

b588e0b

rebase master

9d38a6e

update synth.metadata

bf85031

update synthtool

ce7afdd

timeout system-test, install sample test

6d44cfe

fix subscriberStub

6d91c8b

fix unit test for subscriberStub

df1138c

fix topicPath

fb8c091

hack synth.py to fix eslint for gts 1.0.0

15369fe

hack synth.py to fix duplicate http namespace

fa27b00

IAM client injection

89342b3

googlebot added cla: no This human has *not* signed the Contributor License Agreement. cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Apr 7, 2020

add .gitignore

bc476b9

summer-ji-eng added 3 commits April 7, 2020 15:29

update synthtool

b4d598e

remove node 8 hack in synth.py, because depends on node 10

a167a7a

remove timeout for test

03f454f

summer-ji-eng requested review from JustinBeckwith and feywind April 8, 2020 00:16

JustinBeckwith suggested changes Apr 8, 2020

View reviewed changes

xiaozhenliu-gg5 reviewed Apr 8, 2020

View reviewed changes

address the comments

4acc3e9

summer-ji-eng requested a review from JustinBeckwith April 8, 2020 19:02

JustinBeckwith approved these changes Apr 8, 2020

View reviewed changes

feywind mentioned this pull request Apr 8, 2020

fix: add a close() method to the generated SubscriberClient for 1.x #948

Merged

feywind approved these changes Apr 9, 2020

View reviewed changes

alexander-fenster reviewed Apr 9, 2020

View reviewed changes

alexander-fenster approved these changes Apr 9, 2020

View reviewed changes

summer-ji-eng merged commit 2fc68ba into master Apr 9, 2020

summer-ji-eng deleted the summer_convert branch April 9, 2020 18:19

release-please Bot mentioned this pull request Apr 9, 2020

chore: release 2.0.0 #946

Closed

This was referenced Apr 29, 2020

chore: release 2.0.0 #985

Closed

chore: release 2.0.0 #989

Closed

This was referenced May 19, 2020

chore: release 2.0.0 #1003

Closed

chore: release 2.0.0 #1005

Merged

release-please Bot mentioned this pull request May 20, 2020

chore: release 2.0.0 #1006

Closed

feywind pushed a commit to feywind/nodejs-pubsub that referenced this pull request Nov 12, 2024

docs: fix some typos (googleapis#923)

80022f8

* docs: fix families typo * docs: fix initial typo


		import {google} from '../proto/iam';
		import {google} from '../protos/protos';

Conversation

summer-ji-eng commented Mar 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Mar 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

googlebot commented Apr 7, 2020

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xiaozhenliu-gg5 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

feywind left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexander-fenster left a comment

Choose a reason for hiding this comment

Uh oh!

JustinBeckwith commented Apr 9, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

summer-ji-eng commented Mar 23, 2020 •

edited

Loading

codecov Bot commented Mar 24, 2020 •

edited

Loading