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

NodeJS example needs a description in the README #728

Closed
markmandel opened this issue Apr 18, 2019 · 3 comments · Fixed by #945
Closed

NodeJS example needs a description in the README #728

markmandel opened this issue Apr 18, 2019 · 3 comments · Fixed by #945
Assignees
Labels
good first issue These are great first issues. If you are looking for a place to start, start here! help wanted We would love help on these issues. Please come help us! kind/cleanup Refactoring code, fixing up documentation, etc kind/documentation Documentation for Agones
Milestone

Comments

@markmandel
Copy link
Member

Context:
https://github.com/GoogleCloudPlatform/agones/tree/master/examples/nodejs-simple

There was some confusion about what this was an example of. We should have a description of it's purpose in the README.md.

Maybe something similar to what we see in the C++ example:
https://github.com/GoogleCloudPlatform/agones/tree/master/examples/cpp-simple

@markmandel markmandel added help wanted We would love help on these issues. Please come help us! good first issue These are great first issues. If you are looking for a place to start, start here! kind/documentation Documentation for Agones kind/cleanup Refactoring code, fixing up documentation, etc labels Apr 18, 2019
@BradfordMedeiros
Copy link
Contributor

BradfordMedeiros commented Apr 27, 2019

So this is tangental to the issue , but a good place for it, some questions:

  1. Are you sure you want to expose the client field externally ? Seems like an implementation detail of the client, not something that should be publically accessible (best would be to solve via scope, but _ is fine too)

And

  1. How do you close the connection? Can you provide a top level close or something equivalent ? I'd rather not touch the internal client . I don't feel I should need to touch that to use the client.

Thanks !

@markmandel
Copy link
Member Author

Apologies - not sure this is the right place. Once the description has been added to the example, this ticket will be closed, and your questions will be lost for all eternity 😃

If there are improvement to the Node SDK itself, please file individual tickets for each issue 👍, then they can be individually tracked and improved on (looks like this is two tickets - or maybe 2 PRs?)

@aLekSer
Copy link
Collaborator

aLekSer commented Jun 26, 2019

This example was really useful to create my SDK harness test on Node JS.
Also note that license is missing also:
index.js

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue These are great first issues. If you are looking for a place to start, start here! help wanted We would love help on these issues. Please come help us! kind/cleanup Refactoring code, fixing up documentation, etc kind/documentation Documentation for Agones
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants