Adding a README.md file for the simple-udp to help developer to get start #229
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Build Succeeded 👏 Build Id: 6a086088-7dae-4d4a-b701-4dd453cda9e1 The following development artifacts have been built, and will exist for the next 30 days:
|
|
So this looks pretty great - I'm wondering though, should this be a QuickStart instead? Maybe something like "edit your first game server (go)", then we could also do the same with C++ etc? WDYT? |
|
Thanks Mark. Done. For the C++, I can work on that and open another PR. |
|
This looks pretty good! @enocom did you want to take a quick peek, since I defer to you on Go things 😄 see if there is anything you think should be added to this? |
|
Build Succeeded 👏 Build Id: 3e85eab9-b22f-483a-a26c-e27a9f0d4497 The following development artifacts have been built, and will exist for the next 30 days:
|
|
Build Succeeded 👏 Build Id: 67ce616b-136c-4b5f-b509-33430c5b1a3b The following development artifacts have been built, and will exist for the next 30 days:
|
markmandel
reviewed
May 29, 2018
| Apply the latest settings to kubernetes container. | ||
|
|
||
| ```bash | ||
| >> gcloud config set container/cluster [CLUSTER_NAME] |
There was a problem hiding this comment.
Wondering if we want to keep the gcloud commands? Shouldn't this already be done in the GKE section of the install documentation, and GKE is not a requisite.
There was a problem hiding this comment.
This may need to be done again before running to docker run or any gcloud command to ensure the cluster name is set correctly. I have a concern that the developer may be working with other cluster name while reading this section.
There was a problem hiding this comment.
SGTM, given the above proviso 👍
|
|
||
| ### Push the image to GCP Registry | ||
| ```bash | ||
| docker push gcr.io/[PROJECT_ID]/agones-udp-server:0.2 |
There was a problem hiding this comment.
What if they are running minikube, or their own registry? We should mention this.
Maybe add something like "..or a registry of your choice, such a hub.docker.com" ?
There was a problem hiding this comment.
Sorry that I am having some hard time to run "minikube" in my current local environment. I won't be able to help out to provide steps for that. I can update the README.md to mention about this.
There was a problem hiding this comment.
Okay cool- I see the comment above. SGTM 👍
docs/edit_first_game_server.md
Outdated
|
|
||
| ### Create a new docker image | ||
| ```bash | ||
| docker build -t udp-gcr.io/[PROJECT_ID]/agones-udp-server:0.2 . |
There was a problem hiding this comment.
Good catch!. That's a typo.
docs/edit_first_game_server.md
Outdated
|
|
||
| 1. Install golang from https://golang.org/doc/install. | ||
| 2. Install Docker from https://www.docker.com/get-docker. | ||
| 3. Install kubectl and gcloud by following the instruction https://github.com/GoogleCloudPlatform/agones. |
There was a problem hiding this comment.
I feel like this should be:
"Follow the install instructions (if you haven't already) at: https://github.com/GoogleCloudPlatform/agones/blob/master/install/README.md"
Since we aren't speccifically driving people to GKE, and we have install docs.
WDYT?
docs/edit_first_game_server.md
Outdated
| ## Modify the code and push another new image | ||
|
|
||
| ### Modify the source code | ||
| Modify the main.go file. |
There was a problem hiding this comment.
Do we want to show a change that can be made? Be explicit in the change? Might be a nicer user experience.
There was a problem hiding this comment.
Yes, you are right.
|
Build Succeeded 👏 Build Id: 3493e4f3-fa3e-40e6-b9d2-76ba76674eb1 The following development artifacts have been built, and will exist for the next 30 days:
|
markmandel
added
kind/documentation
g-ericso
commented
May 29, 2018
docs/edit_first_game_server.md
Outdated
| ## Modify the code and push another new image | ||
|
|
||
| ### Modify the source code | ||
| Modify the main.go file. |
There was a problem hiding this comment.
Yes, you are right.
|
Build Succeeded 👏 Build Id: fe44da75-a7c4-4caf-8df0-d776b0ae33b2 The following development artifacts have been built, and will exist for the next 30 days:
|
|
Build Succeeded 👏 Build Id: e8481caa-fb6b-46fa-bbcf-e256b309c45e The following development artifacts have been built, and will exist for the next 30 days:
|
|
@markmandel As long as we squash all the commits, this work looks good to me! |
markmandel
reviewed
May 29, 2018
docs/edit_first_game_server.md
Outdated
|
|
||
| ## Prerequisites | ||
|
|
||
| 1. Active GCP Project. |
There was a problem hiding this comment.
Doesn't the user still need go?
docs/edit_first_game_server.md
Outdated
| @@ -0,0 +1,98 @@ | |||
| # Getting Started | |||
| The following instruction is to help any new developer to use this simple-udp example as the custom agones server template without much docker or kubernetes experience. | |||
There was a problem hiding this comment.
I think this reads better (and fixes some capitalisation issues):
"The following guide is for developers without Docker or Kubernetes experience, that want to use the simple-udp example as a starting point for a custom game server."
docs/edit_first_game_server.md
Outdated
|
|
||
| ## Prerequisites | ||
|
|
||
| 1. Active GCP Project. |
There was a problem hiding this comment.
Expand to "Google Cloud Platforn (GCP)" - much like you did later on for GKE.
docs/edit_first_game_server.md
Outdated
|
|
||
|
|
||
| ### Build Server | ||
| Since docker image is using alpine osx, the "go build" command has to include few more environment variables. |
There was a problem hiding this comment.
docker => Docker
alpine osx => Alpine Linux
docs/edit_first_game_server.md
Outdated
|
|
||
| ## Using Docker File without using Minikube | ||
|
|
||
| The following instruction is about how to use Docker image and push to the gcp without Minikube. For using Minikube with this server example, please try this your own and provide us feedback. |
There was a problem hiding this comment.
"The following are instructions for Google Kubernetes Engine. We welcome a Pull Request to expand this to include Minikube as well"
^ I think that reads better, WDYT?
This line might also be better just after the sentence in "Getting Started" - WDYT?
|
|
||
| ### Push the image to GCP Registry | ||
| ```bash | ||
| docker push gcr.io/[PROJECT_ID]/agones-udp-server:0.2 |
There was a problem hiding this comment.
Okay cool- I see the comment above. SGTM 👍
| Apply the latest settings to kubernetes container. | ||
|
|
||
| ```bash | ||
| >> gcloud config set container/cluster [CLUSTER_NAME] |
There was a problem hiding this comment.
SGTM, given the above proviso 👍
|
Build Succeeded 👏 Build Id: e60ce22b-565c-4429-963a-bfe5e7e34f4b The following development artifacts have been built, and will exist for the next 30 days:
|
|
Awesome. This LGTM, just needs to be rebased down to a single commit (see Contributing Guide), and I'll approve and merge! |
|
Thanks both of you. But GitHub can let you select squash all commits into one after clicking the merge option. Can we do that instead? |
|
We do a rebase and merge strategy for PRs - which means we get the ability to freely use "update branch" without getting a really yuck git history. The downside being that we have to ask people to rebase to a single commit. |
Semver should have 3 digits, so we should be releasing 0.2.0 next.
Added missing "\" at the end of command options.
Moved the installation documentation into the `install` directory and on release, zip up the install instructions and yaml files such that users can download and use them.
This PR does two things: 1) Ensure the built images always have the release registry 2) Ensure the release branch is pushed to upstream, rather than to a forked origin.
Realised there were a couple of steps missing from the release checklist.
g-ericso
force-pushed
the
g-ericso-simple-udp-readme
branch
from
66c5334 to
6b40c9c
Compare
|
I just squashed my commits. Please check if LGTY. |
|
Build Succeeded 👏 Build Id: 88d2148c-b674-401c-b442-4654211c9f5c The following development artifacts have been built, and will exist for the next 30 days:
|
|
Build Failed 😱 Build Id: 1662833f-0df1-47e4-81e4-a464843f2bea Build Logs |
|
Tried to merge from master but bot failed. I am not sure what I have done wrong. Please help. |
|
I'm not sure what you managed to do there 😱 I would suggest pulling down |
|
Closed this PR and open another one #234 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adding a README.md file for the simple-udp to help any developer to get start quicker. The instruction focus on how to customize the main.go file and deploy to GKE.