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

Upgrade Protobuf Dependency #338

Closed
wants to merge 14 commits into from
Closed

Upgrade Protobuf Dependency #338

wants to merge 14 commits into from

Conversation

wozz
Copy link
Contributor

@wozz wozz commented May 24, 2020

  • Upgrades protobuf dependency to be compatible with the new v2 golang protobuf API.
  • Switches to go modules dependency tracking, drops vendoring
  • Updates WORKSPACE name
  • Drops support of gogo due to lack of support for the new API and GoGo Protobuf looking for new ownership gogo/protobuf#691

mwoz-sc added 14 commits May 26, 2020 00:28
Signed-off-by: michael.wozniak <michael.wozniak@snapchat.com>
Signed-off-by: michael.wozniak <michael.wozniak@snapchat.com>
Signed-off-by: michael.wozniak <michael.wozniak@snapchat.com>
Signed-off-by: michael.wozniak <michael.wozniak@snapchat.com>
Signed-off-by: michael.wozniak <michael.wozniak@snapchat.com>
Signed-off-by: michael.wozniak <michael.wozniak@snapchat.com>
Signed-off-by: michael.wozniak <michael.wozniak@snapchat.com>
Signed-off-by: michael.wozniak <michael.wozniak@snapchat.com>
Signed-off-by: michael.wozniak <michael.wozniak@snapchat.com>
Signed-off-by: michael.wozniak <michael.wozniak@snapchat.com>
Signed-off-by: michael.wozniak <michael.wozniak@snapchat.com>
Signed-off-by: michael.wozniak <michael.wozniak@snapchat.com>
Signed-off-by: michael.wozniak <michael.wozniak@snapchat.com>
Signed-off-by: michael.wozniak <michael.wozniak@snapchat.com>
@akonradi
Copy link
Contributor

Dropping GoGo support seems like it merits discussion. @kyessenov, you requested and added support for GoGo proto, what are your thoughts?

@wozz
Copy link
Contributor Author

wozz commented May 26, 2020

FWIW, go-control-plane no longer uses gogo protos as of envoyproxy/go-control-plane#226

@Mythra
Copy link

Mythra commented May 26, 2020

This seems to also be a lot of changes in one, which is concerning incase we run into problems with just one of the changes that we weren’t expecting, or for the ones that require more discussion like gogo support (I realize go-control-plane doesn’t use it anymore, but do others? Do they need it for a bit longer to migrate off?)

Is there any particular reason we can’t separate these to say:

  • one for upgrading to protov2
  • one for dropping gogo support
  • one for changing the local build to go mod?

@wozz
Copy link
Contributor Author

wozz commented May 26, 2020

sure, I can work on separating the changes, but they will probably need a specific order. I kind of worked backwards on this from my goal of adding proto v2 support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants