-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Javaupdate #9254
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
base: master
Are you sure you want to change the base?
Javaupdate #9254
Conversation
Summary of ChangesHello @harshyyy21, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request updates the Firebase CLI's handling of Java versions for emulators. It raises the minimum recommended Java version to 21 and changes the behavior for unsupported versions from a hard failure to a deprecation warning, ensuring users are informed about future requirements while maintaining current functionality. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request updates the Java version check for emulators, raising the minimum recommended version to 21 and changing the enforcement from an error to a warning. The changes are generally in the right direction, but I've found a critical issue where the deprecation warning is not correctly propagated, rendering part of the change ineffective. I've also suggested improvements to the changelog entry for accuracy and to the code for better style and readability. Please review the comments for details.
@@ -1,3 +1,4 @@ | |||
- Marks Java 11 and below as deprecated. Support will be dropped in Firebase CLI v15. Please upgrade to Java version 21 or above to continue using the emulators. |
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.
The changelog entry is inaccurate. The code changes deprecate Java versions < 21
, not just 11 and below
. To avoid confusion, please update the changelog to reflect this. A more active phrasing would also improve readability.
For example: Deprecated Java versions below 21. Support will be dropped in Firebase CLI v15. Please upgrade to Java version 21 or above to continue using the emulators.
- Marks Java 11 and below as deprecated. Support will be dropped in Firebase CLI v15. Please upgrade to Java version 21 or above to continue using the emulators. | |
- Deprecated Java versions below 21. Support will be dropped in Firebase CLI v15. Please upgrade to Java version 21 or above to continue using the emulators. |
export const JAVA_DEPRECATION_WARNING = | ||
"firebase-tools no longer supports Java versions before 11. " + | ||
"Please install a JDK at version 11 or above to get a compatible runtime."; | ||
"firebase-tools will drop support for Java version < 21 soon in firebase-tools@15. " + | ||
"Please install a JDK at version 21 or above to get a compatible runtime."; |
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 improved readability, you can combine this into a single string on one line, removing the need for concatenation.
export const JAVA_DEPRECATION_WARNING = | |
"firebase-tools no longer supports Java versions before 11. " + | |
"Please install a JDK at version 11 or above to get a compatible runtime."; | |
"firebase-tools will drop support for Java version < 21 soon in firebase-tools@15. " + | |
"Please install a JDK at version 21 or above to get a compatible runtime."; | |
export const JAVA_DEPRECATION_WARNING = | |
"firebase-tools will drop support for Java version < 21 soon in firebase-tools@15. Please install a JDK at version 21 or above to get a compatible runtime."; |
Description
This PR checks the Java version before starting emulators that require Java and warns users if the Java version < 21.
Scenarios Tested
Tested this with Java 11 and Java 25.
Java 11 output:
Java 25 output: