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

Define the proto definition for the allocator service #1025

Merged
merged 1 commit into from
Sep 5, 2019

Conversation

pooneh-m
Copy link
Contributor

@pooneh-m pooneh-m commented Aug 30, 2019

Define the v1alpha1 gRPC API protobuf definition for the allocator service according to the GameServerAllocation Extension API server.
issue: #597

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: f93996f1-e4b3-4fa7-a76d-c771e4776340

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

@pooneh-m
Copy link
Contributor Author

pooneh-m commented Aug 30, 2019

Upgrading the protobuf package to fix the build error for proto3 dependency: #1027

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.

Looks like this issue is the linter is failing on the generated code. I think you will need to have a line "This code was autogenerated. Do not edit directly." - which is included in https://github.com/googleforgames/agones/blob/master/build/boilerplate.go.txt

May be easier to get all the make commands working in this PR?

@@ -0,0 +1,539 @@
// Code generated by protoc-gen-go. DO NOT EDIT.
Copy link
Member

Choose a reason for hiding this comment

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

This will need a licence header as well. We have examples in the sdk gen tooling on how we did this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like this issue is the linter is failing on the generated code. I think you will need to have a line "This code was autogenerated. Do not edit directly." - which is included in https://github.com/googleforgames/agones/blob/master/build/boilerplate.go.txt

May be easier to get all the make commands working in this PR?

Thanks, yes I am working on the make commands. Will update this PR when done.

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 53546aba-8738-4c69-ba4d-77c7ca812182

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

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 0cd0daef-cc21-4031-a64c-f1614ea55be9

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

@markmandel
Copy link
Member

Just checking this is good for review - happy to do so, but noticed it's been pared back to just the proto. Wasn't sure if more was coming.

@pooneh-m
Copy link
Contributor Author

pooneh-m commented Sep 4, 2019

To keep the reviews focused, I broke down the PR to two, one for the proto and one for the generated code and the scripts for code generation. I'll send the latter PR by the end of today.

@markmandel markmandel added area/user-experience Pertaining to developers trying to use Agones, e.g. SDK, installation, etc kind/feature New features for Agones and removed area/user-experience Pertaining to developers trying to use Agones, e.g. SDK, installation, etc labels Sep 4, 2019
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.

Approved!

I'd prefer it without that TODO 😄 , for reasons outlined below, but if you want to leave it in, merge away! 👍

cmd/allocator/v1alpha1/allocation.proto Show resolved Hide resolved
cmd/allocator/v1alpha1/allocation.proto Outdated Show resolved Hide resolved
@google-oss-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: markmandel, pooneh-m
To complete the pull request process, please assign
You can assign the PR to them by writing /assign in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@google-oss-robot
Copy link

New changes are detected. LGTM label has been removed.

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: e2d99487-262a-4f52-8718-ccfa0dd4e97a

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/1025/head:pr_1025 && git checkout pr_1025
  • helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.0.0-c620106

…o the gameserverallocation Extention API server.
@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: ab726cdb-362c-4e7a-b577-4a5ef71193ab

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/1025/head:pr_1025 && git checkout pr_1025
  • helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.0.0-a39df5b

@pooneh-m pooneh-m merged commit acaccf0 into googleforgames:master Sep 5, 2019
@pooneh-m pooneh-m deleted the grpc branch September 6, 2019 17:24
@markmandel markmandel added this to the 1.0.0 milestone Sep 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature New features for Agones size/M
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants