Alexander Wels has posted comments on this change.

Change subject: engine: root.war cleanup
......................................................................


Patch Set 13:

(9 comments)

....................................................
File 
backend/manager/modules/branding/src/main/java/org/ovirt/engine/core/branding/BrandingManager.java
Line 127:      * @param type a string naming the application type to load the 
theme for.
Line 128:      * @return A {@code List} of {@code BrandingTheme}s.
Line 129:      */
Line 130:     public synchronized List<BrandingTheme> getBrandingThemes(final 
String type) {
Line 131:         this.applicationType = type;
You make a good point, and I will make it happen.
Line 132:         if (themes == null && brandingRootPath.exists() && 
brandingRootPath.isDirectory()
Line 133:                 && brandingRootPath.canRead()) {
Line 134:             themes = new ArrayList<BrandingTheme>();
Line 135:             List<File> directories = Arrays.asList(


....................................................
File 
backend/manager/modules/branding/src/main/java/org/ovirt/engine/core/branding/BrandingTheme.java
Line 170:      * Get the style sheet type based on the passed in {@code 
ApplicationType}.
Line 171:      * @param type The application type to get the style sheet string 
for.
Line 172:      * @return A {@code String} representation of the style sheet 
name.
Line 173:      */
Line 174:     public String getThemeStyleSheet(String applicationType) {
Actually we don't need the application type in the constructor, as you point 
out it is never used anywhere. So I removed it there. This application type is 
used to filter the messages available in the theme to the correct section.
Line 175:         return brandingProperties.getProperty(applicationType + 
CSS_POST_FIX);
Line 176:     }
Line 177: 
Line 178:     /**


Line 331:         return null;
Line 332:     }
Line 333: 
Line 334:     @Override
Line 335:     public String toString() {
See above
Line 336:         return "Path to theme: " + getPath() + ", User portal css: " 
//$NON-NLS-1$ //$NON-NLS-2$
Line 337:         + getThemeStyleSheet("userportal") + ", Web admin css: " 
//$NON-NLS-1$ //$NON-NLS-2$
Line 338:         + getThemeStyleSheet("webadmin") + ", Welcome page css: " 
//$NON-NLS-1$ //$NON-NLS-2$
Line 339:         + getThemeStyleSheet("welcome"); //$NON-NLS-1$


....................................................
File 
backend/manager/modules/root/src/main/java/org/ovirt/engine/core/DocsServlet.java
Line 63:                 if (!languagePageShown) {
Line 64:                     setLangPageShown(response, true);
Line 65:                     request.setAttribute(LocaleFilter.LOCALE, locale);
Line 66:                     request.setAttribute(ENGLISH_HREF, redirect);
Line 67:                     final ServletContext forwardContext = 
getServletContext().getContext(localeDocsMissing);
Any suggestions on what to do if it is null? besides die horribly?
Line 68:                     final RequestDispatcher dispatcher = 
forwardContext.getRequestDispatcher(localeDocsMissing);
Line 69:                     dispatcher.forward(request, response);
Line 70:                 } else {
Line 71:                     //Redirect to English version of the document


Line 64:                     setLangPageShown(response, true);
Line 65:                     request.setAttribute(LocaleFilter.LOCALE, locale);
Line 66:                     request.setAttribute(ENGLISH_HREF, redirect);
Line 67:                     final ServletContext forwardContext = 
getServletContext().getContext(localeDocsMissing);
Line 68:                     final RequestDispatcher dispatcher = 
forwardContext.getRequestDispatcher(localeDocsMissing);
Any suggestions on what to do if it is null? besides die horribly?
Line 69:                     dispatcher.forward(request, response);
Line 70:                 } else {
Line 71:                     //Redirect to English version of the document
Line 72:                     response.sendRedirect(redirect);


....................................................
File 
backend/manager/modules/root/src/main/java/org/ovirt/engine/core/WelcomeServlet.java
Line 23
Line 24
Line 25
Line 26
Line 27
That is a matter of taste, in this case to be consistent with the rest of the 
applications it made a lot of sense to move everything into the webapp 
descriptor.


....................................................
File 
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/servlet/PageNotFoundForwardServlet.java
Line 69:     @Override
Line 70:     protected void doGet(final HttpServletRequest request, final 
HttpServletResponse response) throws IOException,
Line 71:         ServletException {
Line 72:         //Forward to the page not found URI of the target context, 
which should have all the proper branding/messages.
Line 73:         final ServletContext forwardContext = 
getServletContext().getContext(targetContext);
Done
Line 74:         final RequestDispatcher dispatcher = 
forwardContext.getRequestDispatcher(pageNotFoundURI);
Line 75:         dispatcher.forward(request, response);
Line 76:     }
Line 77: 


Line 70:     protected void doGet(final HttpServletRequest request, final 
HttpServletResponse response) throws IOException,
Line 71:         ServletException {
Line 72:         //Forward to the page not found URI of the target context, 
which should have all the proper branding/messages.
Line 73:         final ServletContext forwardContext = 
getServletContext().getContext(targetContext);
Line 74:         final RequestDispatcher dispatcher = 
forwardContext.getRequestDispatcher(pageNotFoundURI);
Done
Line 75:         dispatcher.forward(request, response);
Line 76:     }
Line 77: 


....................................................
File 
frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/GwtDynamicHostPageServlet.java
Line 70:     public static final String ETAG_HEADER = "Etag"; //$NON-NLS-1$
Line 71: 
Line 72:     private static final String HOST_JSP = "/GwtHostPage.jsp"; 
//$NON-NLS-1$
Line 73:     private static final String UTF_CONTENT_TYPE = "text/html; 
charset=UTF-8"; //$NON-NLS-1$
Line 74:     private static final String APPLICATION_TYPE = "applicationType"; 
//$NON-NLS-1$
Turns out I don't really need to do any of this, check out the latest patch 
when I push it.
Line 75: 
Line 76:     private BackendLocal backend;
Line 77: 
Line 78:     private ObjectMapper mapper;


-- 
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: 13
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: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Einav Cohen <eco...@redhat.com>
Gerrit-Reviewer: Greg Sheremeta <gsher...@redhat.com>
Gerrit-Reviewer: Juan Hernandez <juan.hernan...@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