jonnybot0 commented on code in PR #310:
URL: https://github.com/apache/groovy-geb/pull/310#discussion_r2920850523


##########
integration/geb-testcontainers/src/main/groovy/grails/plugin/geb/support/delegate/DownloadSupportDelegate.groovy:
##########
@@ -0,0 +1,173 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *    https://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+package grails.plugin.geb.support.delegate
+
+import geb.download.DownloadSupport
+import grails.plugin.geb.ContainerGebSpec
+import groovy.transform.CompileStatic
+import groovy.transform.SelfType
+
+/**
+ * Handles delegation to the DownloadSupport instance so that the Geb API can 
be used directly in the test.
+ * <p>
+ * As method parameter names are not available in the Geb artifacts we are 
delegating manually to
+ * get the best possible IDE support and user experience.
+ *
+ * @author Mattias Reichel
+ * @since 4.2
+ */
+@CompileStatic
+@SelfType(ContainerGebSpec)
+trait DownloadSupportDelegate implements DownloadSupport {

Review Comment:
   I suspect that the traits in this package may be something we could simplify 
and remove for the Geb library. If the Grails test containers integration wants 
to keep doing things this way, that's totally fine. For the Geb library, my 
intuition is that we can provide a simpler base class for people to build off.
   
   I recognize, you're probably following this pattern in no small part to 
provide continuity with the old Grails plugin, so no complaints. This also may 
be something that's better shown than told. I'm going to try to come up with an 
alternate implementation and open a PR into your branch to demonstrate what I 
mean. If I can't get it working, you'll know that it was just me overthinking! 
😄 



-- 
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]

Reply via email to