See if java 21 presubmits will pass #173985
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the default Java version from 17 to 21 for Linux, Mac, and Windows builds in the engine's CI configuration, with the aim of resolving build failures. The changes in .ci.yaml correctly bump the version number. My review includes a suggestion to use YAML anchors to define the Java version in a single place, which would improve the maintainability of this configuration file. As a potential follow-up, it would be beneficial to review and rename related files that reference the old Java version in their names (e.g., dev/devicelab/bin/tasks/android_java17_dependency_smoke_tests.dart) to maintain consistency across the codebase.
| dependencies: >- | ||
| [ | ||
| {"dependency": "open_jdk", "version": "version:17"}, | ||
| {"dependency": "open_jdk", "version": "version:21"}, |
There was a problem hiding this comment.
Since the Java version is updated in three places, consider using a YAML anchor to define it once. This would improve maintainability by ensuring consistency and simplifying future updates, especially if this experimental change is made permanent. 1
For example, you could define a java_version anchor:
# At the top of the file, perhaps in a 'vars' section
vars:
java_version: &java_version "version:21"And then reference it where needed:
{"dependency": "open_jdk", "version": *java_version}Style Guide References
Footnotes
1 participant
We are seeing failures because java is not installed on builds where java should be installed.
https://ci.chromium.org/ui/p/flutter/builders/try/Linux%20Engine%20Drone/3520859/overview
Thread on this topic.
https://chat.google.com/room/AAAAm69vf-M/dDN5s5vjedA
Hypothesis is that java is installed but some builds need java 21 instead of 17.
Related to #173986
Pre-launch Checklist
///).