chore: bake Java formatter into image#3271
Conversation
product-auto-label
bot
added
size: l
product-auto-label
bot
added
size: s
| ARG OWLBOT_CLI_COMMITTISH=ac84fa5c423a0069bbce3d2d869c9730c8fdf550 | ||
| ARG PROTOC_VERSION=25.5 | ||
| ARG GRPC_VERSION=1.67.1 | ||
| ARG JAVA_FORMAT_VERSION=1.7 |
There was a problem hiding this comment.
I think we can upgrade the formatter, the latest version is 1.23.0.
It will cause code formatting so it's better in a separate PR.
There was a problem hiding this comment.
Agree that it should be tracked differently. However, I think the newer versions of formatter are not compatible with Java 8.
There was a problem hiding this comment.
This fomatter is not part of client library and we should be able use a new version of Java to run it.
There was a problem hiding this comment.
Since it is a jar, I think it still needs to be run with Java. I see that we are currently using Java 21 in our build file, but we probably want to use a lower version to guarantee the compatibility since our client library still supports Java 8.
There was a problem hiding this comment.
I think this repo has a check to verify the compiled code is compatible with Java 8 (maven profile should make sure of that). Of course, we can change it to Java8 as well.
On the other hand, running generator/formatter jar only requires JRE which I think we can choose a different version from JDK. We can optimize the image to only include a JRE but not a JDK.
product-auto-label
bot
added
size: m
| RUN ln -sf ${NODE_PATH}/* /usr/local/bin | ||
|
|
||
| # download the Java formatter | ||
| ADD https://maven-central.storage-download.googleapis.com/maven2/com/google/googlejavaformat/google-java-format/${JAVA_FORMAT_VERSION}/google-java-format-${JAVA_FORMAT_VERSION}-all-deps.jar \ |
There was a problem hiding this comment.
This probably need to be downloaded from Airlock as well, but definitely a separate PR. cc: @suztomo.
| if [[ ! -f "${required_tool}" ]]; then | ||
| >&2 echo "File ${required_tool} not found in the " | ||
| >&2 echo "filesystem. Please configure your environment and store the " | ||
| >&2 echo "generator jar in this location" |
There was a problem hiding this comment.
Since this logic is reused now, we should update the wording from "generator jar" to something more generic.
There was a problem hiding this comment.
We probably don't need this sh anymore once we move to hermetic build image in showcase right? Or at least it should be much simpler, things like downloading jars would not be needed anymore.
There was a problem hiding this comment.
Yes, I'll cleanup these files when I'm working on b/325061217
|
|
In this PR: - Bake Java formatter into image. --------- Thank you for opening a Pull Request! Before submitting your PR, please read our [contributing guidelines](https://github.com/googleapis/gapic-generator-java/blob/main/CONTRIBUTING.md). There are a few things you can do to make sure it goes smoothly: - [ ] Make sure to open an issue as a [bug/issue](https://github.com/googleapis/gapic-generator-java/issues/new/choose) before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea - [ ] Ensure the tests and linter pass - [ ] Code coverage does not decrease (if any source code was changed) - [ ] Appropriate docs were updated (if necessary) Fixes #<issue_number_goes_here> ☕️ Co-authored-by: Joe Wang <106995533+JoeWang1127@users.noreply.github.com> Co-authored-by: cloud-java-bot <cloud-java-bot@google.com>
#3271 renders this volume mapping unnecessary
googleapis/sdk-platform-java#3271 renders this volume mapping unnecessary
3 participants
In this PR: