Copilot commented on code in PR #15598:
URL: https://github.com/apache/grails-core/pull/15598#discussion_r3137316996
##########
grails-wrapper/src/main/java/grails/init/GrailsVersion.java:
##########
@@ -138,29 +190,17 @@ public static GrailsVersion getPreferredGrailsVersion() {
System.exit(1);
}
- if (!properties.containsKey("grailsVersion")) {
+ if (!properties.containsKey(GRAILS_VERSION_PROPERTY)) {
return null;
}
- String grailsVersion = properties.getProperty("grailsVersion");
- if (grailsVersion == null) {
- String overrideGrailsVersion =
System.getenv("PREFERRED_GRAILS_VERSION");
- if (overrideGrailsVersion != null) {
- try {
- return new GrailsVersion(overrideGrailsVersion);
- } catch (Exception e) {
- System.out.println("An invalid Grails Version [" +
overrideGrailsVersion + "] was specified in PREFERRED_GRAILS_VERSION");
- e.printStackTrace();
- System.exit(1);
- }
- }
-
- System.out.println("gradle.properties does not contain
grailsVersion; assuming latest Grails Version");
+ String grailsVersion = properties.getProperty(GRAILS_VERSION_PROPERTY);
+ if (grailsVersion == null || grailsVersion.trim().isEmpty()) {
return null;
Review Comment:
`readVersionFromProperties` treats a present-but-blank/whitespace
`grailsVersion` value as “not set” and will then allow
`PREFERRED_GRAILS_VERSION` (or latest-from-metadata) to take over. This doesn’t
match the stated precedence in the Javadoc (“gradle.properties containing a
grailsVersion key”). Consider treating blank/whitespace as an error, or
updating the precedence/Javadoc to explicitly require a non-blank value and
define behavior when the key exists but is empty.
```suggestion
System.out.println("A blank Grails Version was specified in
gradle.properties for key [" + GRAILS_VERSION_PROPERTY + "]");
System.exit(1);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
