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

chore: upgrade rules_go to 0.49.0 #1569

Merged
merged 3 commits into from
Sep 3, 2024

Conversation

quartzmo
Copy link
Member

  • upgrade bazel_gazelle to 0.38.0
  • upgrade com_google_protobuf to 25.4
  • add rules_proto 6.0.2
  • remove -use_generic_streams_experimental
    • fix error: protoc-gen-go-grpc: no such flag -use_generic_streams_experimental

refs: #1554

* upgrade bazel_gazelle to 0.38.0
* upgrade com_google_protobuf to 25.4
* add rules_proto 6.0.2
* remove -use_generic_streams_experimental
  * fix error: protoc-gen-go-grpc: no such flag -use_generic_streams_experimental

refs: googleapis#1554
@quartzmo quartzmo requested review from a team as code owners August 29, 2024 23:58
@noahdietz
Copy link
Collaborator

remove -use_generic_streams_experimental

  • fix error: protoc-gen-go-grpc: no such flag -use_generic_streams_experimental

We will need to take the "breaking change" introduced by this if we want to remove it. The change applies to our showcase testing stuff, so not that big of a deal tbh.

grpc-go went from a stream client interface, to a Go "generic" type, which apidiff flags as an incompat change in the grpc code we generate on the fly for showcase.

@quartzmo
Copy link
Member Author

@noahdietz You mean we just need to merge this PR in spite of the failing apidiff, correct? Or do you think the merge commit should flag the breaking change with a bang, and that the release notes should mention it?

@codyoss
Copy link
Member

codyoss commented Aug 30, 2024

@quartzmo have you tried to add this change to googleapis to see if it is compatible there?

@quartzmo
Copy link
Member Author

I ran bazelisk clean --expunge && bazelisk build //google/cloud/asset/v1:asset_go_gapic locally against latest googleapis with googleapis/WORKSPACE pointed at local branch for this PR, is that what you mean?

@quartzmo
Copy link
Member Author

@noahdietz My local protoc was outdated, which is why I got the error no such flag -use_generic_streams_experimental and mistakenly assumed the flag had been removed. Restored.

@codyoss
Copy link
Member

codyoss commented Aug 30, 2024

@quartzmo I would try to build the whole package(so other langs too). The reason I ask is because our workspace in googleapis explicitly sets the version of some of these deps. But I think this change would cause them to raise:

@quartzmo quartzmo force-pushed the bazel-upgrade-rules-go branch from 1b8ea21 to 4903ea7 Compare August 30, 2024 21:43
@quartzmo
Copy link
Member Author

@codyoss Thanks for pointing that out. I reverted the updates to com_google_protobuf and bazel_gazelle, leaving only the io_bazel_rules_go update.

@quartzmo quartzmo merged commit 64df153 into googleapis:main Sep 3, 2024
7 checks passed
@quartzmo quartzmo deleted the bazel-upgrade-rules-go branch September 3, 2024 14:35
@quartzmo quartzmo mentioned this pull request Sep 3, 2024
1 task
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.

3 participants