Juan Hernandez has posted comments on this change.

Change subject: userportal, webadmin: Added caching
......................................................................


Patch Set 12: (4 inline comments)

What is the reason to put all the new content in a new frontend-overlay 
project. We already include the frontend.jar file generated by the frontend 
project inside the WEB-INF/lib directory of both user portal and webadmin. 
Can't we put all the new content (classes, .jsp, web-fragment.xml, etc) in the 
frontend project?

....................................................
File frontend/webadmin/modules/frontend-overlay/pom.xml
Line 59:         </configuration>
Line 60:       </plugin>
Line 61:     </plugins>
Line 62:   </build>
Line 63: </project>
Some of the classes in this project import classes from the GWT packages, but 
there is no dependency for GWT here. This produces a compilation failure, at 
least in my environment.

I had to add the following in order to compile successfully:

  <dependency>
    <groupId>com.google.gwt</groupId>
    <artifactId>gwt-servlet</artifactId>
  </dependency>
  <dependency>
    <groupId>com.google.gwt</groupId>
    <artifactId>gwt-user</artifactId>
  </dependency>

The gwt-user artifact should not be needed, as we don't import anything 
directly, but the POM for gwt-servlet is incorrect, so we need to include 
gwt-user explicitly. It shouldn't ideally be here, but in the 
dependencyManagement section of the root POM, where we already have gwt-servlet.


....................................................
File 
frontend/webadmin/modules/frontend-overlay/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/GenericApiGWTServiceImpl.java
Line 20: import org.ovirt.engine.core.common.queries.VdcQueryType;
Line 21: import org.ovirt.engine.core.common.users.VdcUser;
Line 22: import org.ovirt.engine.ui.frontend.gwtservices.GenericApiGWTService;
Line 23: 
Line 24: import com.google.gwt.rpc.server.RpcServlet;
This import produces a compilation failure as there is no dependency on GWT 
inside the POM. See the comment in the POM.
Line 25: 
Line 26: public class GenericApiGWTServiceImpl extends RpcServlet implements 
GenericApiGWTService {
Line 27: 
Line 28:     private static final long serialVersionUID = 7395780289048030855L;


....................................................
File 
frontend/webadmin/modules/frontend-overlay/src/main/resources/META-INF/web-fragment.xml
Line 35: 
Line 36:        <servlet>
Line 37:                <servlet-name>UserPortalHostPageServlet</servlet-name>
Line 38:                
<servlet-class>org.ovirt.engine.ui.frontend.server.gwt.UserPortalHostPageServlet</servlet-class>
Line 39:        </servlet>
It is my understanding that the fragment should contain only what is common to 
userportal and web admin, so shouldn't this go on only in the web.xml file of 
the user portal? Same for the classes that are specific to user portal and 
webadmin, shouldn't they go in the corresponding projects?
Line 40: 
Line 41:        <servlet>
Line 42:                <servlet-name>WebAdminHostPageServlet</servlet-name>
Line 43:                
<servlet-class>org.ovirt.engine.ui.frontend.server.gwt.WebAdminHostPageServlet</servlet-class>


Line 40: 
Line 41:        <servlet>
Line 42:                <servlet-name>WebAdminHostPageServlet</servlet-name>
Line 43:                
<servlet-class>org.ovirt.engine.ui.frontend.server.gwt.WebAdminHostPageServlet</servlet-class>
Line 44:        </servlet>
Same, but for webadmin.
Line 45: 
Line 46:        <servlet>
Line 47:                <servlet-name>GenericApiServlet</servlet-name>
Line 48:                
<servlet-class>org.ovirt.engine.ui.frontend.server.gwt.GenericApiGWTServiceImpl</servlet-class>


--
To view, visit http://gerrit.ovirt.org/10449
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5d8e02ae542a4aa37bd421bde5582c0f3e9820ad
Gerrit-PatchSet: 12
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alexander Wels <[email protected]>
Gerrit-Reviewer: Alexander Wels <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Einav Cohen <[email protected]>
Gerrit-Reviewer: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to