-
-
Notifications
You must be signed in to change notification settings - Fork 422
Remove ignored Configuration GRPC API #558
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
Conversation
8956017
to
6f42ae3
Compare
@cmaglie rebased on master to include latest changes and re-generated proto files!
go.mod
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this appengine keeps coming back? Are we using it?
I can't approve my own PR, anyway the rebase seems ok, we can merge IMHO.
Ok to remove because we're 0.x, but the reason why I deprecated it instead of killing with fire was to avoid breaking clients that were not using the Config message (it was the case of the PRO ide).
At the moment we have the
Configuration
field inInitReq
that is marked as deprecated but it's actually not supported anymore.IMHO if we want to deprecate it we should not silently ignore it otherwise, from the client perspective, it seems that the command has been accepted, leading to a hard to diagnose errors.
Otherwise, if we don't want to support it anymore it's better to merge this PR and remove it altogether so we have a clear indicator that the old API is no longer available.