deps: move from github.com/golang/protobuf to google.golang.org/protobuf/proto #6919
Conversation
d.CheckValid() already does the nil check Co-authored-by: Arvind Bright <arvind.bright100@gmail.com>
d.CheckValid() already does the nil check Co-authored-by: Arvind Bright <arvind.bright100@gmail.com>
|
@dfawley please check this PR. It might need another code review just to be sure that after merging with the new updates, it is still ok. |
test/tools/tools.go
Outdated
| @@ -27,7 +27,8 @@ package tools | |||
|
|
|||
| import ( | |||
| _ "github.com/client9/misspell/cmd/misspell" | |||
| _ "github.com/golang/protobuf/protoc-gen-go" | |||
| _ "golang.org/x/lint/golint" | |||
There was a problem hiding this comment.
This might not be required anymore. We removed usage of golint here
| spb "google.golang.org/genproto/googleapis/rpc/status" | ||
| "google.golang.org/grpc/codes" | ||
| "google.golang.org/protobuf/proto" |
There was a problem hiding this comment.
@Clement-Jean -- This seems to be still unresolved?
|
A few comments @Clement-Jean -- PTAL. While you are taking a look, could you also look at the merge conflict? |
|
@arvindbr8 I'm not entirely sure about the "unresolved" comment. What did you mean? @dfawley There seem to be still some work to do to remove the old package. Could you take a look at the remaining usages (channelz, credentials, internal/pretty, and xds/internal/xdsclient/bootstrap) and tell me if any of these can be translated with the protoadapt package? If yes, I'll work on that. |
I think he meant |
Oh true, there is still more to do. I have a huge PR reworking channelz, so please still wait on that. But the other ones should be fine if you are willing to work on them. Thank you! |
|
The remaining mention to github.com/golang/protobuf seems to be indirect dependency, some mentions in scripts and channelz. Could you take a look at the changes? especially in reflection/grpc_testing_not_regenerate, I'm not sure if it is correct. I splitted the testv3.go into two files (pb and grpc) but if you need to merge them let me know. |
|
Would you mind reverting all those changes since the last review so we can submit the already-reviewed code without delay? You can move them into another branch and we can go from there. Thanks! |
Clement-Jean
force-pushed
the
remove_old_proto_pkg
branch
from
ea987eb
to
b837411
Compare
|
@dfawley This should be reverted |
arvindbr8
changed the title
|
Hi, I don't know whether there is a third phase planned to this task, but in my application there is one remaining place where i see the deprecated dependency being brought in. Credentials.go in google.golang.org/grpc/credentials ( grpc-go/credentials/credentials.go Line 31 in 5ccf176 |
|
@RJKeevil We cannot remove that just yet because of possible conflicts due to a rewrite. However it's planned and I will work on it ASAP 😊 |
|
Yes, I think we're all clear now to finish it. Unfortunately, it looks like we'll still keep the dependency in perpetuity because of testing in https://github.com/grpc/grpc-go/blob/cce163274b6cb9de2b2bf0bc742384e5755fee42/reflection/grpc_testing_not_regenerate/testv3.go#L41C9-L41C41 |
This is the second part of #6736 which is moving away from github.com/golang/protobuf in favor of google.golang.org/protobuf/proto
RELEASE NOTES: