-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Conversation
@@ -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" |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…er, monitoring and speech. (#11148)
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.