Alon Bar-Lev has posted comments on this change.

Change subject: engine: no documentation message
......................................................................


Patch Set 6:

(7 comments)

Starting to understand!!!

....................................................
File 
backend/manager/modules/root/src/main/java/org/ovirt/engine/core/DocsMissingServlet.java
I may be wrong... long time since I developed anything for the web.

but if you just put jsp in the war it will just work... no need for this 
redirection servlet.

and if so important to have this APPLICATION_TYPE attribute, you can pass init 
parameter to jsp as any servlet.

and... let's say this is required, the NO_LOCALE_JSP can be init parameter so 
we can reuse.

but I really don't think we need this servlet.
Line 1: package org.ovirt.engine.core;
Line 2: 
Line 3: import java.io.IOException;
Line 4: 


....................................................
File backend/manager/modules/root/src/main/webapp/WEB-INF/help/no_lang.jsp
Line 1: <%@ page pageEncoding="UTF-8" session="false" %>
Line 2: <%@ taglib prefix="c" uri="http://java.sun.com/jsp/jstl/core"; %>
Line 3: <%@ taglib prefix="fmt" uri="http://java.sun.com/jsp/jstl/fmt"; %>
Line 4: <fmt:setBundle basename="org.ovirt.engine.core.docswarning" var="docs"/>
I suggest bundle to be "root" per the root war, so that all messages of root be 
in the same bundle.
Line 5: <!DOCTYPE html>
Line 6: <html>
Line 7: <head>
Line 8:     <meta http-equiv="Content-type" content="text/html; charset=utf-8" 
/>


Line 5: <!DOCTYPE html>
Line 6: <html>
Line 7: <head>
Line 8:     <meta http-equiv="Content-type" content="text/html; charset=utf-8" 
/>
Line 9:     <link rel="shortcut icon" 
href="${pageContext.request.contextPath}/ovirt-engine-theme-resource/favicon" 
type="image/x-icon" />
the icon paste can be in some utility so we can reuse.
Line 10:     <title><fmt:message key="docs.missing.title" bundle="${docs}">
Line 11:                <fmt:param 
value="${requestScope['locale'].getDisplayLanguage()}" />
Line 12:            </fmt:message>
Line 13:     </title>


Line 13:     </title>
Line 14:     <c:if test="${requestScope['brandingStyle'] != null}">
Line 15:         <c:forEach items="${requestScope['brandingStyle']}" 
var="theme">
Line 16:             <c:if 
test="${theme.getThemeStyleSheet(requestScope['applicationType']) != null}">
Line 17:     <link rel="stylesheet" type="text/css" 
href="${pageContext.request.contextPath}/ovirt-engine-theme${theme.path}/${theme.getThemeStyleSheet(requestScope['applicationType'])}">
where the theme is (/ovirt-engine-theme) should be init parameter or better 
application context attribute. In other words, it should be only specified at 
web.xml.
Line 18:             </c:if>
Line 19:         </c:forEach>
Line 20:     </c:if>
Line 21: </head>


Line 16:             <c:if 
test="${theme.getThemeStyleSheet(requestScope['applicationType']) != null}">
Line 17:     <link rel="stylesheet" type="text/css" 
href="${pageContext.request.contextPath}/ovirt-engine-theme${theme.path}/${theme.getThemeStyleSheet(requestScope['applicationType'])}">
Line 18:             </c:if>
Line 19:         </c:forEach>
Line 20:     </c:if>
hmmm... can't the above be in some common utility?
Line 21: </head>
Line 22: <body>
Line 23:     <div>
Line 24:         <div class="obrand_left">


....................................................
File backend/manager/modules/root/src/main/webapp/WEB-INF/web.xml
Line 65:       <param-name>file</param-name>
Line 66:       <param-value>%{ENGINE_MANUAL}</param-value>
Line 67:     </init-param>
Line 68:     <init-param>
Line 69:         <param-name>forwardPath</param-name>
forward? maybe missing?
Line 70:         <param-value>/ovirt-engine/docsmissing</param-value>
Line 71:     </init-param>
Line 72:   </servlet>
Line 73:   <servlet-mapping>


Line 200: 
Line 201:   <filter-mapping>
Line 202:     <filter-name>BrandingFilter</filter-name>
Line 203:     <url-pattern>/ovirt-engine</url-pattern>
Line 204:     <url-pattern>/ovirt-engine/docsmissing</url-pattern>
instead of each location, can we have a URI namespace?

I know this is part of bug#, but no reason to force people to write explicit 
servlets.
Line 205:     <dispatcher>FORWARD</dispatcher>
Line 206:     <dispatcher>REQUEST</dispatcher>
Line 207:   </filter-mapping>
Line 208: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iceae0ebf671efc951522c464db1a5b2b95dd5637
Gerrit-PatchSet: 6
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alexander Wels <aw...@redhat.com>
Gerrit-Reviewer: Alexander Wels <aw...@redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: Einav Cohen <eco...@redhat.com>
Gerrit-Reviewer: Greg Sheremeta <gsher...@redhat.com>
Gerrit-Reviewer: Vojtech Szocs <vsz...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to