-
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: make generator version an optional param #3040
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 was hoping we can skip calling the generator if the version is not provided, maybe reusing the proto_only
flag, but it probably requires more effort to populate the value and pass the flag around. I think this approach if good for now.
@@ -104,6 +107,7 @@ docker run \ | |||
-u "$(id -u):$(id -g)" \ | |||
-v "$(pwd):${workspace_name}" \ | |||
-v "$HOME"/.m2:/home/.m2 \ | |||
-e generator_version="${generator_version}" \ |
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.
From our shell style guide: Constants and anything exported to the environment should be capitalized, separated with underscores, and declared at the top of the file.
The top of the file part doesn't apply, but I think it's reasonable to make it uppercase for consistency with other env vars we set up in the Dockerfile
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.
Makes sense, do you mind updating it in the absence of Joe?
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:
gapic_generator_version
as an optional parameter.gapic_generator_version
through environment variable if it's not specified in the generation config.