jamesfredley commented on code in PR #15654:
URL: https://github.com/apache/grails-core/pull/15654#discussion_r3314873611
##########
grails-async/core/src/main/groovy/org/grails/async/transform/internal/DelegateAsyncTransformation.java:
##########
@@ -200,7 +201,7 @@ protected DelegateAsyncTransactionalMethodTransformer
lookupAsyncTransactionalMe
try {
Class<?> transformerClass =
getClass().getClassLoader().loadClass("org.grails.async.transform.internal.DefaultDelegateAsyncTransactionalMethodTransformer");
return (DelegateAsyncTransactionalMethodTransformer)
transformerClass.getDeclaredConstructor().newInstance();
- } catch (Throwable e) {
+ } catch (Exception ignored) {
Review Comment:
Your reasoning here is exactly the rationale captured in #15682 (PR-B) - the
catch-Throwable was bad style and your explanation is cited in the PR body.
Extracted there for atomic review. Reverted from PR-A in ba235da8f7.
##########
grails-async/plugin/src/main/groovy/grails/async/services/PersistenceContextPromiseDecorator.groovy:
##########
@@ -20,8 +20,8 @@ package grails.async.services
import groovy.transform.CompileStatic
-import grails.persistence.support.PersistenceContextInterceptorExecutor
import grails.async.decorator.PromiseDecorator
+import grails.persistence.support.PersistenceContextInterceptorExecutor
Review Comment:
Reverted from PR-A in ba235da8f7 - the import-order reformat is now in
#15682 (PR-B) along with the rest of the async cleanup. Will return through the
next merge of 8.0.x once #15682 lands.
##########
grails-async/plugin/src/main/groovy/grails/async/web/AsyncGrailsWebRequest.groovy:
##########
@@ -19,23 +19,25 @@
package grails.async.web
-import groovy.transform.CompileStatic
-import org.grails.web.util.GrailsApplicationAttributes
-import org.grails.web.servlet.mvc.GrailsWebRequest
-import org.springframework.context.ApplicationContext
-import org.springframework.util.Assert
-import org.springframework.web.context.request.async.AsyncWebRequest
-
import java.util.concurrent.atomic.AtomicBoolean
import java.util.function.Consumer
+import groovy.transform.CompileStatic
Review Comment:
Reverted from PR-A in ba235da8f7 - the import-order change is in #15682
(PR-B).
##########
grails-data-mongodb/core/src/test/groovy/grails/mongodb/cascade/MongoCascadeSpec.groovy:
##########
@@ -18,12 +18,13 @@
*/
package grails.mongodb.cascade
+import org.apache.grails.data.mongo.core.MongoDatastoreSpec
+
import org.apache.grails.data.mongo.core.GrailsDataMongoTckManager
-import org.apache.grails.data.testing.tck.base.GrailsDataTckSpec
-class MongoCascadeSpec extends GrailsDataTckSpec<GrailsDataMongoTckManager> {
+class MongoCascadeSpec extends MongoDatastoreSpec {
Review Comment:
Extracted to #15685 (PR-E: MongoDatastoreSpec base class + mongo package
rename) - the MongoDatastoreSpec refactor plus the prerequisite mongo `tests/`
-> `specs/` rename was reverted from PR-A in ba235da8f7. Your question 'was the
refactor needed?' can be debated atomically there. Keeping this thread open
until #15685 is resolved.
--
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]
