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

Rename PostAllocate to Allocate in Multi Cluster Allocation Service #1331

Closed
markmandel opened this issue Feb 10, 2020 · 5 comments · Fixed by #1513
Closed

Rename PostAllocate to Allocate in Multi Cluster Allocation Service #1331

markmandel opened this issue Feb 10, 2020 · 5 comments · Fixed by #1513
Assignees
@pooneh-m
Labels
kind/breaking Breaking change kind/cleanup Refactoring code, fixing up documentation, etc
Milestone
1.6.0

Comments

@markmandel
Copy link
Member

This came up in #1314 (comment)

In this service:
https://github.com/googleforgames/agones/blob/master/proto/allocation/v1alpha1/allocation.proto#L22

We agreed that Allocate would be a nicer name to use.
@markmandel markmandel added kind/cleanup Refactoring code, fixing up documentation, etc kind/breaking Breaking change labels Feb 10, 2020
@markmandel
Copy link
Member Author

/cc @pooneh-m for visibility

@markmandel markmandel mentioned this issue Feb 10, 2020
Merged
@pooneh-m pooneh-m self-assigned this Feb 10, 2020
@markmandel
Copy link
Member Author

Thought I had - we could retain and deprecate PostAllocate for a release or two, and keep it alongside the new Allocate Service, to give people a smoother transition before removing it.

@markmandel markmandel mentioned this issue Apr 25, 2020
Closed
@pooneh-m
Copy link
Contributor

pooneh-m commented May 4, 2020

The API is marked as alpha, which means there is no backward compatibility guarantee. This rpc method name change will not be the only breaking change and either we should support every breaking change or this will not be an exception. Because for V1 version the backward compatibility between alpha and v1 will not be carried over, I recommend breaking the API now and support backward compatibility on a stable version release.

@pooneh-m pooneh-m mentioned this issue May 4, 2020
Merged
@markmandel
Copy link
Member Author

That's also fair. I would put it into the "best effort" bucket - if it's easy to do, awesome, if not, I don't have a strong opinion to maintain backward compatibility.

@pooneh-m
Copy link
Contributor

pooneh-m commented May 4, 2020

It is easy to do, but IMHO it is unnecessary.

@markmandel markmandel added this to the 1.6.0 milestone May 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@pooneh-m pooneh-m

Labels
kind/breaking Breaking change kind/cleanup Refactoring code, fixing up documentation, etc
Projects
None yet
Milestone
1.6.0
Development

Successfully merging a pull request may close this issue.

Change rpc method from PostAllocate to Allocate pooneh-m/agones
2 participants
@markmandel @pooneh-m