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 <[email protected]>
Gerrit-Reviewer: Alexander Wels <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Einav Cohen <[email protected]>
Gerrit-Reviewer: Greg Sheremeta <[email protected]>
Gerrit-Reviewer: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
