Copilot commented on code in PR #16155:
URL: https://github.com/apache/grails-core/pull/16155#discussion_r3790543072
##########
grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/proxy/GroovyProxyFactory.groovy:
##########
@@ -70,8 +71,8 @@ class GroovyProxyFactory implements ProxyFactory {
}
}
- @groovy.transform.CompileDynamic
- protected Serializable getIdDynamic(obj) {
+ @CompileDynamic
+ protected static Serializable getIdDynamic(obj) {
Review Comment:
Changing this `protected` helper from an instance method to `static` is a
source/binary breaking change for any downstream subclasses that override it
(static methods are not polymorphic). If this change was only to satisfy
IntelliJ’s “method may be static” inspection, prefer keeping it as an instance
method and suppressing the inspection (there is precedent in the repo, e.g.
`grails-core/src/main/groovy/grails/boot/GrailsBanner.groovy:107`).
This issue also appears in the following locations of the same file:
- line 125
- line 165
--
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]
