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.

Sign up for GitHub

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

Jump to bottom

Add support to create a development gameserver. #558

Merged
merged 1 commit into from
Feb 9, 2019
Merged

Add support to create a development gameserver. #558

merged 1 commit into from
Feb 9, 2019

Conversation

jeremyje
Copy link
Contributor

@jeremyje jeremyje commented Feb 4, 2019

Implementation of #469

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 2610f9d9-e10a-4dfe-903c-82d35973896c

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 2b8fe6f8-ac27-4072-bb35-6939b2b6d880

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 92373bb5-0ec3-45b3-9ec2-0eaea3e72d97

The following development artifacts have been built, and will exist for the next 30 days:

A preview of the website will exist for the next 30 days:

To install this version:

  • git fetch https://github.com/GoogleCloudPlatform/agones.git pull/558/head:pr_558 && git checkout pr_558
  • helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=0.8.0-e8450c2

markmandel
markmandel reviewed Feb 5, 2019
View reviewed changes
Copy link
Member

@markmandel markmandel left a comment

Choose a reason for hiding this comment

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

Overall, this looks pretty good except for a few minor things.

But I think we need some documentation 😄
https://agones.dev/site/docs/contribute/

examples/simple-udp/dev-gameserver.yaml Outdated Show resolved Hide resolved
pkg/gameservers/controller.go Show resolved Hide resolved
@markmandel markmandel added kind/feature New features for Agones area/user-experience Pertaining to developers trying to use Agones, e.g. SDK, installation, etc labels Feb 5, 2019
@markmandel markmandel added this to the 0.8.0 milestone Feb 5, 2019
@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: fe5e0303-02f1-4ca6-86ff-53d11478325b

The following development artifacts have been built, and will exist for the next 30 days:

A preview of the website will exist for the next 30 days:

To install this version:

  • git fetch https://github.com/GoogleCloudPlatform/agones.git pull/558/head:pr_558 && git checkout pr_558
  • helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=0.8.0-83b2921

markmandel
markmandel reviewed Feb 6, 2019
View reviewed changes
site/content/en/docs/Guides/development-server.md Outdated
@@ -0,0 +1,38 @@
---
Copy link
Member

Choose a reason for hiding this comment

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

Let's add a publishDate: 2018-02-19 to the top content so that it doesn't show up before the stable release comes out.

@reductor can you take a look, I'm not sure the phrasing here is right. Maybe it should be something more like:
"Coordinate Agones with a local development server" or something? I'm not quite sure what the documentation should say -- I figured you might have some good words.

We aren't actually hosting it in Agones - we're more coordinating it with a local build?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It also turns out that we need a bunch of the pod spec template stuff to make this all work I'm adding comments for now to say it's ignored. I've been trying to experiment with not requiring it but I have to rip out a lot of validation that's in the yaml files to inch towards that. I'm now experimenting with just putting placeholder values for now.

Copy link
Member

Choose a reason for hiding this comment

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

Saying it's ignored seems reasonable 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

The only thing that causes a bit of confusion I believe is the phrase "Development mode" which makes it seem like its a feature toggle within all of Agones (even though it is in some ways). An alternative might be "Development game server"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What should this feature be called in the docs?

  • Development Mode
  • Unmanaged Mode
  • Local Mode

etc?

Copy link
Contributor

Choose a reason for hiding this comment

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

If using the word "mode" then probably worth associating it with the game server for example "Development Game Server Mode" this saves confusion that its a "mode" on the larger agones installation

Copy link
Member

Choose a reason for hiding this comment

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

This is a "Guide" - which is a howto, essentially, so thinking of it as an action - i.e. what is the user trying to do here?

It doesn't need a "mode" per se - it just needs to describe what we are doing.

  • "Inject local Game Server into Agones"
  • "Point Agones at local game server"
  • "Manage local game server with Agones"

Something like that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Register Local Game Server with Agones?

Copy link
Member

Choose a reason for hiding this comment

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

I like it! 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 1fc453d8-06e8-4353-84c4-b748e5ae1216

The following development artifacts have been built, and will exist for the next 30 days:

A preview of the website will exist for the next 30 days:

To install this version:

  • git fetch https://github.com/GoogleCloudPlatform/agones.git pull/558/head:pr_558 && git checkout pr_558
  • helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=0.8.0-125f443

@markmandel
Copy link
Member

Assuming this all passes, this LGTM. The only thing would be, to please rebase this down to a single commit.
https://github.com/GoogleCloudPlatform/agones/blob/master/CONTRIBUTING.md#submitting-code-via-pull-requests

👍 👍 👍 👍 👍 👍

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: a713b411-0886-4234-b53e-693194654071

The following development artifacts have been built, and will exist for the next 30 days:

A preview of the website (the last 30 builds are retained):

To install this version:

  • git fetch https://github.com/GoogleCloudPlatform/agones.git pull/558/head:pr_558 && git checkout pr_558
  • helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=0.8.0-83be852

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 216ccb4f-4090-47c2-b6d3-f1f5b9fabb25

The following development artifacts have been built, and will exist for the next 30 days:

A preview of the website (the last 30 builds are retained):

To install this version:

  • git fetch https://github.com/GoogleCloudPlatform/agones.git pull/558/head:pr_558 && git checkout pr_558
  • helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=0.8.0-7e94b24

@markmandel markmandel merged commit 95bb039 into googleforgames:master Feb 9, 2019
@markmandel markmandel mentioned this pull request Feb 14, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aLekSer aLekSer aLekSer left review comments

@reductor reductor reductor left review comments

@markmandel markmandel markmandel approved these changes

Assignees
No one assigned
Labels
area/user-experience Pertaining to developers trying to use Agones, e.g. SDK, installation, etc kind/feature New features for Agones
Projects
None yet
Milestone
0.8.0
Development

Successfully merging this pull request may close these issues.
5 participants
@jeremyje @agones-bot @markmandel @reductor @aLekSer