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

Remove gogo support #340

Merged
merged 11 commits into from
Jun 9, 2020
Merged

Remove gogo support #340

merged 11 commits into from
Jun 9, 2020

Conversation

wozz
Copy link
Contributor

@wozz wozz commented May 26, 2020

Splitting up #338 into smaller changes. This change removes support for gogo.

The main reason to drop support is so that protoc-gen-validate can be updated to use the new protobuf API, which gogo does not support. Additionally, it seems up in the air if gogo will continue to be maintained: gogo/protobuf#691

If support is later added for gogo and maintenance picks back up, gogo support can be added back to protoc-gen-validate.

These changes are built on top of #339

mwoz-sc added 6 commits May 26, 2020 10:26
Signed-off-by: michael.wozniak <[email protected]>
Signed-off-by: michael.wozniak <[email protected]>
Signed-off-by: michael.wozniak <[email protected]>
Signed-off-by: michael.wozniak <[email protected]>
Signed-off-by: michael.wozniak <[email protected]>
@kyessenov
Copy link
Contributor

I'm fine dropping gogo since I found it difficult to maintain personally. I am not aware of other users of protoc-gen-validate gogo feature.

@akonradi
Copy link
Contributor

akonradi commented Jun 8, 2020

SGTM. @wozz can you merge master?

@akonradi
Copy link
Contributor

akonradi commented Jun 9, 2020

Sorry, looks like merging #343 introduced a few more conflicts. Once more, please!

@wozz
Copy link
Contributor Author

wozz commented Jun 9, 2020

@akonradi updated

should I just close #339 and do both of these together?

@akonradi
Copy link
Contributor

akonradi commented Jun 9, 2020

No, please keep them separate. It looks like there are still some go.mod and go.sum files added in this PR. Is that intentional?

@wozz
Copy link
Contributor Author

wozz commented Jun 9, 2020

This PR is not quite independent of #339, it is on top of #339, so I was assuming it would be merged after that one.

@akonradi
Copy link
Contributor

akonradi commented Jun 9, 2020

Thanks, I had the order reversed in my head. I'll go ahead and take a look over there then

@akonradi akonradi merged commit c7c8be4 into bufbuild:master Jun 9, 2020
@akonradi
Copy link
Contributor

akonradi commented Jun 9, 2020

Thanks!

rvolosatovs added a commit to TheThingsIndustries/protoc-gen-validate that referenced this pull request Aug 7, 2020
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