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

feat!: Remove orphaned protoc generated files from bigquerydatatransfer, monitoring and speech. #11148

Merged
merged 4 commits into from
Sep 20, 2024

Conversation

blakeli0
Copy link
Contributor

@blakeli0 blakeli0 commented Sep 16, 2024

There are some protoc generated files are not updated in over 3 years, which made them not compatible with latest protobuf runtime anymore. Since the source proto of these files are already deleted, we should delete them from our client library as well.
See go/incompatible-protoc-files-java-sdk for more details.

@blakeli0 blakeli0 marked this pull request as ready for review September 20, 2024 19:57
@blakeli0 blakeli0 requested a review from a team as a code owner September 20, 2024 19:57
@blakeli0 blakeli0 changed the title chore: Remove orphaned protoc generated files from bigquerydatatransfer, monitoring and speech. feat: Remove orphaned protoc generated files from bigquerydatatransfer, monitoring and speech. Sep 20, 2024
@blakeli0 blakeli0 changed the title feat: Remove orphaned protoc generated files from bigquerydatatransfer, monitoring and speech. feat!: Remove orphaned protoc generated files from bigquerydatatransfer, monitoring and speech. Sep 20, 2024
@@ -20,52 +20,7 @@ deep-remove-regex:
- "/java-bigquerydatatransfer/samples/snippets/generated"

deep-preserve-regex:
- "/java-bigquerydatatransfer/google-.*/src/test/java/com/google/cloud/.*/v.*/it/IT.*Test.java"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to delete this IT?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is actually no integration tests for bigquerydatatransfer, so this is like a not-directly-related clean up. I can revert this as well if it is causing confusion.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah ok. If there are no ITs to being with then it doesn't matter. I'm fine with removing

Comment on lines -484 to -487
grpc-google-cloud-speech-v1beta1:2.44.0:2.45.0-SNAPSHOT
grpc-google-cloud-speech-v1p1beta1:2.44.0:2.45.0-SNAPSHOT
proto-google-cloud-speech-v1:4.44.0:4.45.0-SNAPSHOT
proto-google-cloud-speech-v1beta1:2.44.0:2.45.0-SNAPSHOT
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

qq, why are these two removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For speech, the whole v1beta1 version does not exist anymore, see googleapis

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't really tell from the diffs on Github, but I'm guessing the entire subfolder is removed in this PR?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I think still see the folders in the branch. Can we just remove the folders in google-cloud-java?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think they should be removed already. Let's merge this in first, I'll create a follow up cleanup PR if the folders still exist.

Copy link
Contributor

@lqiu96 lqiu96 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@blakeli0 blakeli0 merged commit 4598c2d into main Sep 20, 2024
31 checks passed
@blakeli0 blakeli0 deleted the remove-orphan-protoc branch September 20, 2024 22:51
lqiu96 pushed a commit that referenced this pull request Sep 23, 2024
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