Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Node example documentation for example migration to grpc-node repository #1154

Merged
merged 3 commits into from Aug 2, 2023

Conversation

murgatroid99
Copy link
Member

@murgatroid99 murgatroid99 commented Jun 22, 2023

The actual migration is occurring in grpc/grpc-node#2474 and grpc/grpc#33522. I used master for the node version because at first the examples will only exist on the master branch. After the next release it should be updated to @grpc/grpc-js@1.9.0.

Copy link
Collaborator

@chalin chalin left a comment

Choose a reason for hiding this comment

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

While I haven't walked through the steps from those two edited pages, but it LGTM.

A few notes:

  • The PHP quickstart makes use of the Node quickstart. There's at least one place in the PHP quickstart where you'll need to update a path:
    Just like we did before, from the `examples/node/dynamic_codegen` directory:
  • Can you create an issue to ensure that params.grpc_vers.node is updated once the repo has an official release. You might want to add the URL to the issue in the config file next to the node entry so that we have some context.

@murgatroid99
Copy link
Member Author

I updated the PHP quickstart, and I also updated the node version in config.yaml. That tag doesn't exist quite yet, but it will very soon.

@murgatroid99
Copy link
Member Author

The node version tag now exists.

Copy link
Collaborator

@chalin chalin left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

@murgatroid99
Copy link
Member Author

I don't have write access to this repository. This is ready to merge, whenever someone who does have write access wants to.

@chalin chalin merged commit de75d84 into grpc:main Aug 2, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants