Skip to content
This repository was archived by the owner on Apr 9, 2021. It is now read-only.

update node docs to use addService#776

Open
mitchdraft wants to merge 1 commit into
grpc:masterfrom
mitchdraft:docs-node-addservice-api
Open

update node docs to use addService#776
mitchdraft wants to merge 1 commit into
grpc:masterfrom
mitchdraft:docs-node-addservice-api

Conversation

@mitchdraft

Copy link
Copy Markdown

small change to the docs now that the node server api has changed

link to api: https://github.com/grpc/grpc-node/blob/master/packages/grpc-native-core/src/server.js#L948

the examples work with or without this change, but the change removes this warning message:

$ node server.js
(node:44083) DeprecationWarning: Server#addProtoService: Use Server#addService instead

@thelinuxfoundation

Copy link
Copy Markdown

Thank you for your pull request. Before we can look at your contribution, we need to ensure all contributors are covered by a Contributor License Agreement.

After the following items are addressed, please respond with a new comment here, and the automated system will re-verify.

Regards,
CLA GitHub bot

@mitchdraft

Copy link
Copy Markdown
Author

CLA signed

@carl-mastrangelo carl-mastrangelo left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change seems fine, @murgatroid99 & @ZhouyihaiDing can you tell me if I should merge?

@ZhouyihaiDing

ZhouyihaiDing commented Nov 13, 2018

Copy link
Copy Markdown

The change seems fine to me too. @murgatroid99 can you please take a look?

@hsaliak hsaliak requested a review from nicolasnoble December 5, 2018 23:10
@hsaliak

hsaliak commented Dec 5, 2018

Copy link
Copy Markdown

@murgatroid99 @nicolasnoble can you please rubber stamp, given that this is an example change it would be best if one of you looked over it.

@thisisnotapril

Copy link
Copy Markdown

@murgatroid99 @nicolasnoble @hsaliak are we OK to merge this one?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants