-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make a version of JavaVersion#getMajorVersion()
, which returns int instead of String
#29176
Conversation
Can you check for our usages on the string version in the codebase to see if we are converting it to an int anywhere? |
I already did; my use case is the first one. Most use cases are concerned about the compatibility of a version, and they don't need the version directly. |
@@ -326,6 +326,15 @@ public String getMajorVersion() { | |||
return String.valueOf(ordinal() + 1); |
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.
馃拝 should this line also use getMajorVersionNumber()
?
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.
Absolutely
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.
Fixed!
...java/src/main/java/org/gradle/api/internal/tasks/compile/AbstractJavaCompileSpecFactory.java
Show resolved
Hide resolved
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.
Yeah, I've significantly overlooked a lot of use-cases, as I've focused on the |
WARN: Based on labels, this pull request addresses notable issue but no changes to release note found. |
The merge queue build has failed. Click here to see all failures. |
e8fe2fc
to
c582e63
Compare
@donat could you re-review |
platforms/core-runtime/java-language-extensions/src/main/java/org/gradle/api/JavaVersion.java
Outdated
Show resolved
Hide resolved
platforms/core-runtime/java-language-extensions/src/main/java/org/gradle/api/JavaVersion.java
Outdated
Show resolved
Hide resolved
WARN: Based on labels, this pull request addresses notable issue but no changes to release note found. |
The 馃У spawning this change: #28281 (comment)
When developing the problem reporting for the compiler, I've noticed that this useful method is missing.
Meanwhile we don't rely on this that much, I think it's a worthwhile addition for the users too.