[PATCH] D89862: [clangd] Give the server information about client's remote index protocol version

Thu Oct 22 06:31:40 PDT 2020

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

In D89862#2346967 <https://reviews.llvm.org/D89862#2346967>, @kbobyrev wrote:

> In D89862#2346645 <https://reviews.llvm.org/D89862#2346645>, @kadircet wrote:
>
>> Regarding versioning of grpc layer. In addition to including a version number in every request, looks like there's the concept of "versioned-services".
>>
>> So we basically change the package name to be versioned, i.e. `package clang.clangd.remote.v1` and every time we make a breaking change, we increment the version number and start a new service (while keeping the old one).
>> Hopefully most of the core pieces will be re-usable, hence this will likely only end up adding a new `service` definition with possibly new reply/request types.
>>
>> That might be more manageable than having a version in every request. It will also make handling a little bit easier, as dispatch will happen in grpc layer and server wouldn't have to perform conditional checks.
>
> Good point! @sammccall should we do the package versioning and update to mitigate potential breaking changes?

Yep, tacking "v1" on this preemptively SGTM.

Note this itself is a breaking change :-)

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89862/new/

https://reviews.llvm.org/D89862