-
Notifications
You must be signed in to change notification settings - Fork 53
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: remove protoc grpc from generation config #3390
Conversation
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 know we said that it's OK to not support running scripts directly. What is the effort to support it? Do we need to write additional code or just need to set DOCKER_PROTOC_VERSION
and DOCKER_GRPC_VERSION
?
I decided to keep the support of running python script directly in development guide. |
@@ -88,24 +88,23 @@ WORKDIR /protoc | |||
RUN source /src/library_generation/utils/utilities.sh \ | |||
&& download_protoc "${PROTOC_VERSION}" "${OS_ARCHITECTURE}" | |||
# we indicate protoc is available in the container via env vars | |||
ENV DOCKER_PROTOC_LOCATION=/protoc | |||
ENV DOCKER_PROTOC_LOCATION=/protoc/protoc/bin |
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.
Is duplicated protoc
expected in the location?
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.
Yes.
Do you think we should reduce the duplicated directory?
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.
Yes, if it is not required, just protoc/bin
is clearer IMO. I think we can unzip to the root folder in the line below?
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.
Done.
Quality Gate passed for 'gapic-generator-java-root'Issues Measures |
Quality Gate passed for 'java_showcase_integration_tests'Issues Measures |
In this PR:
protoc_version
andgrpc_version
from generation config.